gpio: 74x164: use a compatible fallback and don't extend the driver
Subsystems (2)
cover gpio: 74x164: use a compatible fallback and don't extend the driver
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Bartosz Golaszewski (2):
Revert "gpio: 74x164: Add On Semi MC74HC595A compat"
dt-bindings: gpio: fairchild,74hc595: use a fallback for Semi
MC74HC595A
.../devicetree/bindings/gpio/fairchild,74hc595.yaml | 10 ++++++---- drivers/gpio/gpio-74x164.c | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-)
Signature · 1 line
2.45.2
PATCH 1/2 Revert "gpio: 74x164: Add On Semi MC74HC595A compat"
From: Bartosz Golaszewski <[email protected]>
This reverts commit b1468db9d865deb5271c9a20d05201b1c0636895.
There's no need to add a new compatible to the driver code, we can handle it with a DT fallback.
--- drivers/gpio/gpio-74x164.c | 2 -- 1 file changed, 2 deletions(-)
drivers/gpio/gpio-74x164.c +0 -2
@@ -165,7 +165,6 @@ static void gen_74x164_remove(struct spi_device *spi)static const struct spi_device_id gen_74x164_spi_ids[] = { { .name ="74hc595" }, - { .name ="74hc595a" }, { .name ="74lvc594" }, {}, };
@@ -174,7 +173,6 @@ MODULE_DEVICE_TABLE(spi, gen_74x164_spi_ids);static const struct of_device_id gen_74x164_dt_ids[] = { { .compatible ="fairchild,74hc595" }, { .compatible ="nxp,74lvc594" }, - { .compatible ="onnn,74hc595a" }, {}, };MODULE_DEVICE_TABLE (of, gen_74x164_dt_ids);
Signature · 1 line
2.45.2
Re: [PATCH 1/2] Revert "gpio: 74x164: Add On Semi MC74HC595A compat"
Quoted 9 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
This reverts commit
b1468db9d865deb5271c9a20d05201b1c0636895.There's no need to add a new compatible to the driver code, we can handle it with a DT fallback.
Signed-off-by Bartosz Golaszewski <[email protected]>---
I assume this was not released anywhere, judging by dates, but would be nice to see confirmation in commit msg or changelog.
drivers/gpio/gpio-74x164.c static void gen_74x164_remove(struct spi_device *spi) · line 168
Quoted 12 lines
drivers/gpio/gpio-74x164.c | 2 -- 1 file changed, 2 deletions(-)drivers/gpio/gpio-74x164.c +0
-1@@ -165,7 +165,6 @@ static void gen_74x164_remove(struct spi_device *spi)static const struct spi_device_id gen_74x164_spi_ids[] = { { .name ="74hc595" }, - { .name ="74hc595a" },
Hm, strictly speaking this is not related. What if some spi-driver-matching uses 74hc595a? There is no compatibility for such matches.
Best regards,
Krzysztof
PATCH 2/2 dt-bindings: gpio: fairchild,74hc595: use a fallback for Semi MC74HC595A
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
This model is 1:1 compatible with fairchild,74hc595 so use the latter as a fallback instead of adding a new stand-alone compatible.
--- .../devicetree/bindings/gpio/fairchild,74hc595.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml +6 -4
@@ -28,10 +28,12 @@ maintainers: properties: compatible: - enum: - - fairchild,74hc595 - - nxp,74lvc594 - - onnn,74hc595a + oneOf: + - const: fairchild,74hc595 + - const: nxp,74lvc594 + - items: + - const: onnn,74hc595a + - const: fairchild,74hc595 reg: maxItems: 1
Signature · 1 line
2.45.2
Re: [PATCH 2/2] dt-bindings: gpio: fairchild,74hc595: use a fallback for Semi MC74HC595A
Quoted 16 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
This model is 1:1 compatible with fairchild,74hc595 so use the latter as a fallback instead of adding a new stand-alone compatible.
Fixes 0ba6cec7acbb ("dt-bindings: gpio: fairchild,74hc595: Add On Semi MC74HC595A compat")Signed-off-by Bartosz Golaszewski <[email protected]>--- .../devicetree/bindings/gpio/fairchild,74hc595.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml +0
-0
Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml maintainers: · line 33
Quoted 10 lines
Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml +3
-4@@ -28,10 +28,12 @@ maintainers: properties: compatible: - enum: - - fairchild,74hc595 - - nxp,74lvc594 - - onnn,74hc595a + oneOf: + - const: fairchild,74hc595 + - const: nxp,74lvc594
Keep enum for these two, preferred pattern.
Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml maintainers: · line 36
Quoted 3 lines
Documentation/devicetree/bindings/gpio/fairchild,74hc595.yaml +3
-0+ - items: + - const: onnn,74hc595a + - const: fairchild,74hc595
Best regards,
Krzysztof
Hi,
Quoted 9 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Is there any reason to introduce a new compatible name at all? Does some pre-existing, widely-used DT blob use it in the wild already? If not, then I don't think it's necessary; for any new boards, their DT's authors should just use the pre-existing names.
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e. generic,74x595, or even generic,spi-shift-register-output.
Quoted 8 lines
Bartosz Golaszewski (2):
Revert "gpio: 74x164: Add On Semi MC74HC595A compat"
dt-bindings: gpio: fairchild,74hc595: use a fallback for SemiMC74HC595A.../devicetree/bindings/gpio/fairchild,74hc595.yaml | 10 ++++++---- drivers/gpio/gpio-74x164.c | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-)
Bence
Quoted 11 lines · Csókás Bence
Hi,
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
Quoted 8 lines
Quoted 2 lines
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Is there any reason to introduce a new compatible name at all? Does some pre-existing, widely-used DT blob use it in the wild already? If not, then I don't think it's necessary; for any new boards, their DT's authors should just use the pre-existing names.
I don't have a strong opinion on this and will defer to DT maintainers but a similar case I'm familiar with is the at24 EEPROM driver where we've got lots of 1:1 compatible chips and we tend to add new compatibles to DT bindings (with fallbacks to associated atmel models) just for the sake of correct HW description in DTS.
Quoted 5 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.
If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
Bart
Quoted 11 lines
Quoted 8 lines
Bartosz Golaszewski (2):
Revert "gpio: 74x164: Add On Semi MC74HC595A compat"
dt-bindings: gpio: fairchild,74hc595: use a fallback for SemiMC74HC595A.../devicetree/bindings/gpio/fairchild,74hc595.yaml | 10 ++++++---- drivers/gpio/gpio-74x164.c | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-)Bence
Hi Bartosz,
Quoted 17 lines · Bartosz Golaszewski
Quoted 8 lines · Csókás Bence
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
Quoted 7 lines
Quoted 2 lines
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Is there any reason to introduce a new compatible name at all? Does some pre-existing, widely-used DT blob use it in the wild already? If not, then I don't think it's necessary; for any new boards, their DT's authors should just use the pre-existing names.
Quoted 5 lines
I don't have a strong opinion on this and will defer to DT maintainers but a similar case I'm familiar with is the at24 EEPROM driver where we've got lots of 1:1 compatible chips and we tend to add new compatibles to DT bindings (with fallbacks to associated atmel models) just for the sake of correct HW description in DTS.
At24 EEPROMs differ from '595 shift registers in that they provide an API with multiple commands, and some commands or parameter bits may differ among different implementations (but usually these differences are called quirks).
All '595 (I'm deliberately writing it like that) shift registers should be 100% compatible, modulo some electrical specifications (voltage levels, maximum speed, power consumption, ...).
Interestingly, the driver is called gpio-74x164.c, while no '164 compatible value is present. Most important difference is that the '164 lacks the output latch, which is used as chip-select with SPI[1].
Quoted 8 lines
Quoted 4 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
For the inverse operation (parallel in, serial out), there's just "pisosr-gpio".
[1] https://www.ovaga.com/blog/transistor/74hc164-vs-74hc595#simple-list-item-2
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
Quoted 45 lines · Geert Uytterhoeven
Hi Bartosz,
Quoted 17 lines · Bartosz Golaszewski
Quoted 8 lines · Csókás Bence
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
Quoted 7 lines
Quoted 2 lines
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Is there any reason to introduce a new compatible name at all? Does some pre-existing, widely-used DT blob use it in the wild already? If not, then I don't think it's necessary; for any new boards, their DT's authors should just use the pre-existing names.
Quoted 5 lines
I don't have a strong opinion on this and will defer to DT maintainers but a similar case I'm familiar with is the at24 EEPROM driver where we've got lots of 1:1 compatible chips and we tend to add new compatibles to DT bindings (with fallbacks to associated atmel models) just for the sake of correct HW description in DTS.
At24 EEPROMs differ from '595 shift registers in that they provide an API with multiple commands, and some commands or parameter bits may differ among different implementations (but usually these differences are called quirks).
All '595 (I'm deliberately writing it like that) shift registers should be 100% compatible, modulo some electrical specifications (voltage levels, maximum speed, power consumption, ...).
Interestingly, the driver is called
gpio-74x164.c, while no '164 compatible value is present. Most important difference is that the '164 lacks the output latch, which is used as chip-select with SPI[1].Quoted 5 lines
Quoted 4 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.
Quoted 7 lines
Quoted 3 lines
If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
For the inverse operation (parallel in, serial out), there's just "pisosr-gpio".
Ok, I admit I don't know the correct next step. I'll wait for Krzysztof, Rob or Conor to chime in (on the subject of representing reality - the actual manufacturer - in DTS) and then possibly just remove patches 1-2 from my tree.
Bartosz
Quoted 22 lines · Bartosz Golaszewski
Quoted 22 lines
At24 EEPROMs differ from '595 shift registers in that they provide an API with multiple commands, and some commands or parameter bits may differ among different implementations (but usually these differences are called quirks).
All '595 (I'm deliberately writing it like that) shift registers should be 100% compatible, modulo some electrical specifications (voltage levels, maximum speed, power consumption, ...).
Interestingly, the driver is called
gpio-74x164.c, while no '164 compatible value is present. Most important difference is that the '164 lacks the output latch, which is used as chip-select with SPI[1].Quoted 8 lines
Quoted 4 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
Quoted 9 lines
Quoted 3 lines
For the inverse operation (parallel in, serial out), there's just "pisosr-gpio".
Ok, I admit I don't know the correct next step. I'll wait for Krzysztof, Rob or Conor to chime in (on the subject of representing reality - the actual manufacturer - in DTS) and then possibly just remove patches 1-2 from my tree.
Well, folks, I don't know the exact device, so maybe there is no point in a new compatible if there is a claim all devices have same interface and documenting all of them would result in 1000 redundant compatibles... but OTOH, that's what we still do with jedec,spi and at24, so if we can add specific compatibles for these, we can do same also here.
Best regards,
Krzysztof
Hi Krzysztof,
Quoted 31 lines · Krzysztof Kozlowski
Quoted 22 lines · Bartosz Golaszewski
Quoted 22 lines
At24 EEPROMs differ from '595 shift registers in that they provide an API with multiple commands, and some commands or parameter bits may differ among different implementations (but usually these differences are called quirks).
All '595 (I'm deliberately writing it like that) shift registers should be 100% compatible, modulo some electrical specifications (voltage levels, maximum speed, power consumption, ...).
Interestingly, the driver is called
gpio-74x164.c, while no '164 compatible value is present. Most important difference is that the '164 lacks the output latch, which is used as chip-select with SPI[1].Quoted 8 lines
Quoted 4 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
Quoted 7 lines
Quoted 2 lines
For the inverse operation (parallel in, serial out), there's just "pisosr-gpio".
Ok, I admit I don't know the correct next step. I'll wait for Krzysztof, Rob or Conor to chime in (on the subject of representing reality - the actual manufacturer - in DTS) and then possibly just remove patches 1-2 from my tree.
Quoted 6 lines
Well, folks, I don't know the exact device, so maybe there is no point in a new compatible if there is a claim all devices have same interface and documenting all of them would result in 1000 redundant compatibles... but OTOH, that's what we still do with jedec,spi and at24, so if we can add specific compatibles for these, we can do same also here.
Except that we don't for jedec,spi, unfortunately[1].
At24 and jedec,spi use a complex programming API, with lots of room for deviation and extension. '595 is a pure hardware part[2]: it is just a shift register (driven by SPI clock and MOSI) with a latch (driven by deasserting SPI chip select), without room for deviation. Anything that behaves differently is not a jelly-bean '595 part.
[1] "[PATCH] dt-bindings: mtd: jedec,spi-nor: Document support for more MT25QU parts'
https://lore.kernel.org/all/363186079b4269891073f620e3e2353cf7d2559a.1669988238.git.geert+renesas@glider.be
[2] Ignoring the rumor that all Microchip I/O expanders are actually pre-programmed PIC microcontrollers ;-)
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
Quoted 50 lines · Geert Uytterhoeven
Hi Krzysztof,
Quoted 31 lines · Krzysztof Kozlowski
Quoted 22 lines · Bartosz Golaszewski
Quoted 22 lines
At24 EEPROMs differ from '595 shift registers in that they provide an API with multiple commands, and some commands or parameter bits may differ among different implementations (but usually these differences are called quirks).
All '595 (I'm deliberately writing it like that) shift registers should be 100% compatible, modulo some electrical specifications (voltage levels, maximum speed, power consumption, ...).
Interestingly, the driver is called
gpio-74x164.c, while no '164 compatible value is present. Most important difference is that the '164 lacks the output latch, which is used as chip-select with SPI[1].Quoted 8 lines
Quoted 4 lines
I'm especially against introducing a new, vendor-specific (On Semi, in this case) name; if we really want to introduce a new compatible, at least make it as generic as possible, i.e.
generic,74x595, or evengeneric,spi-shift-register-output.If anything, that would have to be the fallback that the driver knows. The first string in the compatible property has to have an actual vendor (I think, I'll let DT maintainers correct me).
Quoted 7 lines
Quoted 2 lines
For the inverse operation (parallel in, serial out), there's just "pisosr-gpio".
Ok, I admit I don't know the correct next step. I'll wait for Krzysztof, Rob or Conor to chime in (on the subject of representing reality - the actual manufacturer - in DTS) and then possibly just remove patches 1-2 from my tree.
Quoted 6 lines
Well, folks, I don't know the exact device, so maybe there is no point in a new compatible if there is a claim all devices have same interface and documenting all of them would result in 1000 redundant compatibles... but OTOH, that's what we still do with jedec,spi and at24, so if we can add specific compatibles for these, we can do same also here.
Except that we don't for jedec,spi, unfortunately[1].
At24 and jedec,spi use a complex programming API, with lots of room for deviation and extension. '595 is a pure hardware part[2]: it is just a shift register (driven by SPI clock and MOSI) with a latch (driven by deasserting SPI chip select), without room for deviation. Anything that behaves differently is not a jelly-bean '595 part.
Quoted 6 lines
[1] "[PATCH] dt-bindings: mtd: jedec,spi-nor: Document support for more MT25QU parts'
https://lore.kernel.org/all/363186079b4269891073f620e3e2353cf7d2559a.1669988238.git.geert+renesas@glider.be[2] Ignoring the rumor that all Microchip I/O expanders are actually pre-programmed PIC microcontrollers ;-)
It's late into the cycle and as it's not clear whether we really need the new compatible or not, I removed the two patches from my queue. Let's revisit it in the next release.
Bartosz
Quoted 10 lines · Csókás Bence
Hi,
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
There were other suggested solutions (for instance: just use the existing compatible for the On Semi variant) but I figured the most common approach is to use a fallback value for 100% compatible models and this is what Rob suggested as well.
Quoted 4 lines
Quoted 2 lines
This reverts the driver change and makes the "onnn,74hc595a" compatible use "fairchild,74hc595" as fallback.
Is there any reason to introduce a new compatible name at all? Does some
Yes, DT bindings preference, see writing bindings. I assume of course this is a separate device. You cannot use other compatible "foo" for some other device "bar".
Best regards,
Krzysztof