Re: [PATCH] gpio: sn54hc595: new driver for GPIO shift registers chipsets

[Rafał Miłecki <zajec5@xxxxxxxxx>]

On 8 December 2014 at 16:03, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> Hi Rafał,
>
> On Mon, Dec 8, 2014 at 3:47 PM, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
>> On 8 December 2014 at 15:41, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>>> On Mon, Dec 8, 2014 at 3:27 PM, Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
>>>> SN54HC595 and SN74HC595 are devices based on shift registers controlled
>>>> with 5 input signals (serial-in) and providing 8 outputs (parallel-out).
>>>>
>>>> They are present on some Broadcom home router boards where manufacturer
>>>> needed few extra GPIOs.
>>>>
>>>> This driver simply uses specified GPIOs to control shift registers and
>>>> registers another GPIO chip. So you can call it a GPIO extender.
>>>>
>>>> Due to the hardware design only output direction is supported. Reading
>>>> values is handled using tiny internal cache.
>>>>
>>>> Signed-off-by: Rafał Miłecki <zajec5@xxxxxxxxx>
>>>> ---
>>>>  .../devicetree/bindings/gpio/gpio-sn54hc595.txt    |  35 ++++
>>>>  drivers/gpio/Kconfig                               |  11 ++
>>>>  drivers/gpio/Makefile                              |   1 +
>>>>  drivers/gpio/gpio-sn54hc595.c                      | 219 +++++++++++++++++++++
>>>
>>> The '595 is already handled by drivers/gpio/gpio-74x164.c.
>>
>> gpio-74x164.c seems to be tight closely to the SPI. In my case it's
>> GPIO-controller '595.
>
> Right. gpio-74x164.c uses the SPI framework, so it will work with any SPI
> master controller, while your driver contains a very simple variant (without any
> timing constraints) of spi-gpio.c, and is limited to connecting to GPIO pins.
>
>> Do you have any other idea how we could handle this?
>
> Your driver does provide OE support, which gpio-74x164 doesn't support.
> Perhaps that can be added to gpio-74x164 instead?

It's not the missing OE support in gpio-74x164 that worries me, but
the whole rest.

I would need to modify gpio-74x164 to:
1) Use another OF table with different entry
2) Use different probe function that doesn't take spi_device parameter
and doesn't do spi setup
3) Extend struct gen_74x164_chip to include GPIOs
4) Use totally different __gen_74x164_write_config

Amount of shared code would be so small I doubt it's worth it. Both
driver are quite trivial (200 or less LOC). Sharing:
1) ~5 lines long __gen_74x164_write_config
2) ~5 lines long gen_74x164_set_value
3) 1 line long gen_74x164_direction_output
makes me feel it's not really worth combining these two drivers into a one.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html