Re: [PATCH] gpio:Add ASMedia 28xx and 18xx GPIO driver

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

czw., 28 maj 2020 o 11:50 Richard Hsu <saraon640529@xxxxxxxxx> napisał(a):
>
> Hi Bartosz Golaszewski and Linux Walleij,
>   I have modified code and created a new patch correctly.
>

Thanks, this looks much better.

Please make the title be: "gpio: asm28xx-18xx: new driver".

> Hi Bjorn Helgass,
>   Thanks for your reply and suggestions.I will explain it more detail.
> 1.Why i select driver's framework based on AMD South Bridge(gpio-amd
> 8111.c)?
>  Our device is similar as it and it is not the specific gpio controller
> like others.We can't use plain pci_driver mechanism,as it is really
> multiple function,main driver that binds to the pci_device is a bus
> driver(descrbed in gpio-8111.c).This driver is almost the same with
> amd8111.The different parts is the gpio register accessing methods
> between io and pci configuration.
>

I have zero knowledge of the PCI sub-system but I'm seeing some PCI
MFD devices in drivers/mfd. Maybe this is what you're looking for?

> 2.What's special about asm28xx that means you need PM stuff when
> almost nobody else does?
>   Main driver of asm28xx is a bus driver.And when no device be added
> on,bridge sometimes will enter power saving state.other
> sub-system or driver can access it after wake it up(ex.proc_bus_pci_
> read() in drivers/pci/Proc.c).
>
> 3.We end up with multiple drivers controlling the device without
> any coordination between them(ex. PM)?
>   This driver just wake it up before accessing specific gpio
> registers.AP(RW) also wake device up before accessing register.
> That might not be a problem for this particular case.
>
> This driver is used for particular condition and work well.And
> driver framework is the same with related AMD8111.It perhaps
> can keep in the same driver framework and i really hope so.
>
> I am grateful for your assistance.
>
> Signed-off-by: Richard Hsu <saraon640529@xxxxxxxxx>
> ---
>  drivers/gpio/Kconfig             |   8 +
>  drivers/gpio/Makefile            |   1 +
>  drivers/gpio/gpio-asm28xx-18xx.c | 278 +++++++++++++++++++++++++++++++
>  3 files changed, 287 insertions(+)
>  mode change 100644 => 100755 drivers/gpio/Kconfig
>  mode change 100644 => 100755 drivers/gpio/Makefile
>  create mode 100755 drivers/gpio/gpio-asm28xx-18xx.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> old mode 100644
> index 1b96169d84f7..0676082efcb7
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -113,6 +113,14 @@ config GPIO_AMDPT
>           driver for GPIO functionality on Promontory IOHub
>           Require ACPI ASL code to enumerate as a platform device.
>
> +config GPIO_ASM28XX
> +       tristate "Asmedia 28XX/18XX GPIO support"
> +       depends on PCI
> +       select GPIO_GENERIC
> +       help
> +         Driver for GPIO functionality on Asmedia 28XX and 18XX PCI-E Bridge.
> +

Please remove all double newlines here and in the driver source code.

I'm not reviewing the rest of the driver on purpose: I'm 90% sure that
this device should be probed like a regular device in the linux driver
model. I'll review the code once we've sorted that out.

Bartosz