Skip to contentSkip to conversation

Docs: iio: ad7191 Correct clock configuration

PATCH In review Ammar Mustafa 4 participants 7 messages Feb 27 – Apr 12

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Docs: iio: ad7191 Correct clock configuration

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.
- Correct CMOS-compatible clock pin from MCLK2 to MCLK1.

---
Changes since v1:
- Instead of using "tied high" or "tied low", change to active or inactive to remove confusion.
- Undo the swap of entries from previous patch.

 Documentation/iio/ad7191.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Documentation/iio/ad7191.rst +3 -3
index 977d4fea14b0..fd6a23ad44fd 100644
@@ -63,11 +63,11 @@ Clock Configuration
 
 The AD7191 supports both internal and external clock sources:
 
-- When CLKSEL pin is tied LOW: Uses internal 4.92MHz clock (no clock property
+- When CLKSEL pin is ACTIVE: Uses internal 4.92MHz clock (no clock property
   needed)
-- When CLKSEL pin is tied HIGH: Requires external clock source
+- When CLKSEL pin is INACTIVE: Requires external clock source
   - Can be a crystal between MCLK1 and MCLK2 pins
-  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Or a CMOS-compatible clock driving MCLK1 pin and MCLK2 left unconnected
   - Must specify the "clocks" property in device tree when using external clock
 
 SPI Interface Requirements
Signature · 1 line
2.43.0
#1 lore raw
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks, this part looks good in the below documentation update.

Quoted 1 line

- Correct CMOS-compatible clock pin from MCLK2 to MCLK1.

I haven't checked driver yet, but is it only for a single component? Can you double check that _all_ supported by the driver have the same in their datasheet(s)?

...

Quoted 8 lines

+- When CLKSEL pin is ACTIVE: Uses internal 4.92MHz clock (no clock property
needed)
-- When CLKSEL pin is tied HIGH: Requires external clock source +- When CLKSEL pin is INACTIVE: Requires external clock source
- Can be a crystal between MCLK1 and MCLK2 pins

Documentation/iio/ad7191.rst +1 -1
-  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Or a CMOS-compatible clock driving MCLK1 pin and MCLK2 left unconnected
   - Must specify the "clocks" property in device tree when using external clock
Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#2 lore raw
Quoted 9 lines · On Sat, 28 Feb 2026 12:50:46 +0200 Andy Shevchenko…
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks, this part looks good in the below documentation update.

Quoted 7 lines
Quoted 1 line

- Correct CMOS-compatible clock pin from MCLK2 to MCLK1.

I haven't checked driver yet, but is it only for a single component? Can you double check that _all_ supported by the driver have the same in their datasheet(s)?

...

Hi Ammar,

Just a quick note to say I'm going to mark this one in patchwork as needing a new version given Andy's questions have been here a while.

Thanks,

Jonathan

Quoted 10 lines
Quoted 9 lines

+- When CLKSEL pin is ACTIVE: Uses internal 4.92MHz clock (no clock property
needed)
-- When CLKSEL pin is tied HIGH: Requires external clock source +- When CLKSEL pin is INACTIVE: Requires external clock source
- Can be a crystal between MCLK1 and MCLK2 pins

Documentation/iio/ad7191.rst +1 -1
-  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Or a CMOS-compatible clock driving MCLK1 pin and MCLK2 left unconnected
   - Must specify the "clocks" property in device tree when using external clock  
#3 lore raw
Quoted 20 lines · Jonathan Cameron
Quoted 9 lines · On Sat, 28 Feb 2026 12:50:46 +0200 Andy Shevchenko…
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks, this part looks good in the below documentation update.

Quoted 7 lines
Quoted 1 line

- Correct CMOS-compatible clock pin from MCLK2 to MCLK1.

I haven't checked driver yet, but is it only for a single component? Can you double check that _all_ supported by the driver have the same in their datasheet(s)?

...

Quoted 20 lines

Hi Ammar,

Just a quick note to say I'm going to mark this one in patchwork as needing a new version given Andy's questions have been here a while.

Thanks,

Jonathan

Quoted 10 lines
Quoted 9 lines

+- When CLKSEL pin is ACTIVE: Uses internal 4.92MHz clock (no clock property
needed)
-- When CLKSEL pin is tied HIGH: Requires external clock source +- When CLKSEL pin is INACTIVE: Requires external clock source
- Can be a crystal between MCLK1 and MCLK2 pins

Documentation/iio/ad7191.rst +1 -1
-  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Or a CMOS-compatible clock driving MCLK1 pin and MCLK2 left unconnected
   - Must specify the "clocks" property in device tree when using external clock  

Hi Jonathon,

I replied to Andy's questionm not sure if I can attach it in mutt for you, but we found that this driver only supports the AD7191 so no other documentation needs to be updated or check for this issue.
Let me know if I need to do anything else to have this patch merged.

Thank you,

Ammar Mustafa

maintainer
#4 lore raw
Quoted 42 lines · On Thu, 26 Mar 2026 18:04:53 -0400 Ammar Mustafa <[email protected]>…
Quoted 20 lines · Jonathan Cameron
Quoted 9 lines · On Sat, 28 Feb 2026 12:50:46 +0200 Andy Shevchenko…
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks, this part looks good in the below documentation update.

Quoted 7 lines
Quoted 1 line

- Correct CMOS-compatible clock pin from MCLK2 to MCLK1.

I haven't checked driver yet, but is it only for a single component? Can you double check that _all_ supported by the driver have the same in their datasheet(s)?

...

Quoted 20 lines

Hi Ammar,

Just a quick note to say I'm going to mark this one in patchwork as needing a new version given Andy's questions have been here a while.

Thanks,

Jonathan

Quoted 10 lines
Quoted 9 lines

+- When CLKSEL pin is ACTIVE: Uses internal 4.92MHz clock (no clock property
needed)
-- When CLKSEL pin is tied HIGH: Requires external clock source +- When CLKSEL pin is INACTIVE: Requires external clock source
- Can be a crystal between MCLK1 and MCLK2 pins

Documentation/iio/ad7191.rst +1 -1
-  - Or a CMOS-compatible clock driving MCLK2 pin
+  - Or a CMOS-compatible clock driving MCLK1 pin and MCLK2 left unconnected
   - Must specify the "clocks" property in device tree when using external clock    
Quoted 7 lines

Hi Jonathon,

I replied to Andy's questionm not sure if I can attach it in mutt for you, but we found that this driver only supports the AD7191 so no other documentation needs to be updated or check for this issue.
Let me know if I need to do anything else to have this patch merged.

Given it's docs, I'll a sneak it in (not so worried if this one gets build time in linux-next). Applied.

Thanks,

Jonathan

Quoted 3 lines

Thank you,

Ammar Mustafa

#5 lore raw
In reply to Andy Shevchenko
Quoted 6 lines · Andy Shevchenko
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks for your patch, which is now commit d2a4ec19d2a2e54c ("Docs:
iio: ad7191 Correct clock configuration") in char-misc-next and iio/togreg.

That commit message still says "inactive" and "high", thus adding to the confustion.

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
maintainer
#6 lore raw
Quoted 13 lines · On Mon, 30 Mar 2026 10:09:28 +0200 Geert Uytterhoeven <[email protected]>…
Quoted 6 lines · Andy Shevchenko
Quoted 3 lines · Ammar Mustafa

Correct the ad7191 documentation to match the datasheet:
- Fix inverted CLKSEL pin logic: device uses external clock when pin is inactive, and internal CMOS/crystal when high.

high --> active

Thanks for your patch, which is now commit d2a4ec19d2a2e54c ("Docs:
iio: ad7191 Correct clock configuration") in char-misc-next and iio/togreg.

Quoted 7 lines

That commit message still says "inactive" and "high", thus adding to the confustion.

Gr{oetje,eeting}s,

Geert

Ah. Sorry, I missed that and can't really do anything to fix it up now :( All got a bit rushed as I was off on holiday.

Jonathan