Hi Stephen,
thanks for your patch!
Overall it looks good I have a few nitpicks
On Sat, Feb 10, 2024 at 8:09 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> The ChromeOS embedded controller (EC) supports setting the state of
> GPIOs when the system is unlocked, and getting the state of GPIOs in all
> cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
> expander. Add a driver to get and set the GPIOs on the EC through the
> host command interface.
>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Bartosz Golaszewski <brgl@xxxxxxxx>
> Cc: Benson Leung <bleung@xxxxxxxxxxxx>
> Cc: Guenter Roeck <groeck@xxxxxxxxxxxx>
> Cc: <linux-gpio@xxxxxxxxxxxxxxx>
> Cc: <chrome-platform@xxxxxxxxxxxxxxx>
> Cc: Pin-yen Lin <treapking@xxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
(...)
> +config GPIO_CROS_EC
> + tristate "ChromeOS EC GPIO support"
> + depends on CROS_EC
> + help
> + GPIO driver for exposing GPIOs on the ChromeOS Embedded
> + Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called gpio-cros-ec.
Classified as "MFD Expander" but I honestly don't know anything better.
> +#include <linux/bitops.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
Do you need init.h?
I guess maybe... I thought module would be enough for this.
> +static int cros_ec_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
> +{
> + if (gpio_pin < chip->ngpio)
> + return 0;
> +
> + return -EINVAL;
> +}
If this check is needed, it should be in gpiolib I think?
> +#ifdef CONFIG_OF
This ifdef should not be needed these days.
> +static const struct of_device_id cros_ec_gpio_of_match[] = {
> + { .compatible = "google,cros-ec-gpio" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_gpio_of_match);
> +#endif
> +
> +static struct platform_driver cros_ec_gpio_driver = {
> + .probe = cros_ec_gpio_probe,
> + .driver = {
> + .name = "cros-ec-gpio",
> + .of_match_table = of_match_ptr(cros_ec_gpio_of_match),
Nor the of_match_ptr() business.
With these fixed/addressed:
Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Yours,
Linus Walleij
