Re: [PATCH 1/3 v3] x86/platform/geode: Convert net5501 LED to GPIO machine descriptor

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi,

On 11/3/20 1:04 AM, Linus Walleij wrote:
> This makes the machine look up the LED from a GPIO machine
> descriptor table. The Geode LEDs should be on the CS5535
> companion chip.
> 
> Cc: linux-gpio@xxxxxxxxxxxxxxx
> Cc: Andres Salomon <dilinger@xxxxxxxxxx>
> Cc: linux-geode@xxxxxxxxxxxxxxxxxxx
> Cc: Darren Hart <dvhart@xxxxxxxxxxxxx>
> Cc: platform-driver-x86@xxxxxxxxxxxxxxx
> Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Linus, in case you did not know yet, I have take over
drivers/platform/x86 maintainership from Andy.

Andy, the MAINTAINERS entry for arch/x86/platform says:

X86 PLATFORM DRIVERS - ARCH
R:      Darren Hart <dvhart@xxxxxxxxxxxxx>
R:      Andy Shevchenko <andy@xxxxxxxxxxxxx>
L:      platform-driver-x86@xxxxxxxxxxxxxxx
L:      x86@xxxxxxxxxx
S:      Maintained
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/core
F:      arch/x86/platform

Andy, so I guess that with your Reviewed-by added, these are expected to
get picked up by the tip tree people ?

Linus, it seems that you did not "Cc: x86@xxxxxxxxxx" which is
listed in MAINTAINERS for these, and is probably necessary to
get these merged through the tip tree.

Note I'm happy to pick these up through:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/

I actually had them in my local review-hans branch before noticing that
that they went to arch/x86/platform. But I've dropped them now as I'm
not sure if merging them through the pdx86 tree is the right thing to do,
the MAINTAINERS file at least suggests things should be done differently.

Linus, if a v4 with "Cc: x86@xxxxxxxxxx" is necessary you may add my:

Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>

To the entire series.

Regards,

Hans





> ---
> ChangeLog v2->v3:
> - Rebase on v5.10-rc1
> - Resend
> ChangeLog v1->v2:
> - Drop excess comma after terminator { }
> - Collect Andy's Reviewed-by
> ---
>  arch/x86/platform/geode/net5501.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/platform/geode/net5501.c b/arch/x86/platform/geode/net5501.c
> index 163e1b545517..558384acd777 100644
> --- a/arch/x86/platform/geode/net5501.c
> +++ b/arch/x86/platform/geode/net5501.c
> @@ -20,6 +20,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/input.h>
>  #include <linux/gpio_keys.h>
> +#include <linux/gpio/machine.h>
>  
>  #include <asm/geode.h>
>  
> @@ -55,9 +56,7 @@ static struct platform_device net5501_buttons_dev = {
>  static struct gpio_led net5501_leds[] = {
>  	{
>  		.name = "net5501:1",
> -		.gpio = 6,
>  		.default_trigger = "default-on",
> -		.active_low = 0,
>  	},
>  };
>  
> @@ -66,6 +65,15 @@ static struct gpio_led_platform_data net5501_leds_data = {
>  	.leds = net5501_leds,
>  };
>  
> +static struct gpiod_lookup_table net5501_leds_gpio_table = {
> +	.dev_id = "leds-gpio",
> +	.table = {
> +		/* The Geode GPIOs should be on the CS5535 companion chip */
> +		GPIO_LOOKUP_IDX("cs5535-gpio", 6, NULL, 0, GPIO_ACTIVE_HIGH),
> +		{ }
> +	},
> +};
> +
>  static struct platform_device net5501_leds_dev = {
>  	.name = "leds-gpio",
>  	.id = -1,
> @@ -80,6 +88,7 @@ static struct platform_device *net5501_devs[] __initdata = {
>  static void __init register_net5501(void)
>  {
>  	/* Setup LED control through leds-gpio driver */
> +	gpiod_add_lookup_table(&net5501_leds_gpio_table);
>  	platform_add_devices(net5501_devs, ARRAY_SIZE(net5501_devs));
>  }
>  
>