Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200114031056.44502-1-pmalani@chromium.org> <b39666db-38e4-c131-bfbb-077a53a8ef69@collabora.com>
 <CACeCKacSiMQOA63c81deGHOvKZb=MEFQqjK=iQRmmot8GrjS+A@mail.gmail.com> <4ddf93f1-fe0c-763c-25d6-144bdce5cd4c@collabora.com>
In-Reply-To: <4ddf93f1-fe0c-763c-25d6-144bdce5cd4c@collabora.com>
From:   Prashant Malani <pmalani@chromium.org>
Date:   Tue, 14 Jan 2020 13:02:22 -0800
Message-ID: <CACeCKafFD94EaiCPkJOosQzfRaApsMnh3ciZyUyNCfczmTxGiw@mail.gmail.com>
Subject: Re: [PATCH v5 1/2] platform: chrome: Add cros-usbpd-notify driver
To:     Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc:     Guenter Roeck <groeck@chromium.org>,
        Benson Leung <bleung@chromium.org>,
        Lee Jones <lee.jones@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Jon Flatley <jflat@chromium.org>,
        Gwendal Grignou <gwendal@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jan 14, 2020 at 12:29 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
>
>
> On 14/1/20 20:23, Prashant Malani wrote:
> > On Tue, Jan 14, 2020 at 9:07 AM Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> >>
> >> Hi Prashant,
> >>
> >> On 14/1/20 4:10, Prashant Malani wrote:
> >>> From: Jon Flatley <jflat@chromium.org>
> >>> +};
> >>> +MODULE_DEVICE_TABLE(acpi, cros_usbpd_acpi_device_ids);
> >> Ops, there is a build error here, please make sure to build the driver=
 before
> >> sending.
> > Hmm. That's odd, I did check  using arm64 defconfig. My apologies.
> > Could you kindly share your build commands so that I can use this
> > going forward?
> >>
> >> s/cros_usbpd_acpi_device_ids/cros_usbpd_notify_acpi_device_ids/
> >>
>
> one problem is the above^
>
> drivers/platform/chrome/cros_usbpd_notify.c:67:27: error:
> =E2=80=98cros_usbpd_acpi_device_ids=E2=80=99 undeclared here (not in a fu=
nction); did you mean
> =E2=80=98cros_usbpd_notify_acpi_device_ids=E2=80=99?
>
> The others I didn't dig in. My build commands are the common.
>
> make ARCH=3Darm64 CROSS_COMPILE=3Daarch64-linux-gnu- defconfig
> # Enable the new cros_usbpd_notify driver
> make ARCH=3Darm64 CROSS_COMPILE=3Daarch64-linux-gnu- -j4
This is helpful. Thanks Enric!
>
> Cheers,
>  Enric
>
> >> Apart from this I'm getting build errors with my defconfig which has C=
ONFIG_ACPI
> >> and CONFIG_OF enabled.
> >>
> >> Another problem that I have with this driver is that actually there is=
 no user
> >> for it. I'd really prefer have this included on a series that also int=
roduces
> >> the user of this notifier. If you are only testing the ACPI case you c=
an focus
> >> first on this use case and we add later the OF case (maybe is an easy =
way to
> >> proceed).
> > I'm testing with an arm64 device too locally. I will try to
> > incorporate the new change which updates cros_usbpd-charger to use the
> > notifier mechanism. I think we can try for the OF case simultaneously
> > (it is probably just the build errors I need to check for)
> >
> > Thanks again!
> >>
> >> Thanks,
> >>  Enric
> >>
> >>> +
> >>> +static struct acpi_driver cros_usbpd_notify_acpi_driver =3D {
> >>> +     .name =3D DRV_NAME,
> >>> +     .class =3D DRV_NAME,
> >>> +     .ids =3D cros_usbpd_notify_acpi_device_ids,
> >>> +     .ops =3D {
> >>> +             .add =3D cros_usbpd_notify_add_acpi,
> >>> +             .notify =3D cros_usbpd_notify_acpi,
> >>> +     },
> >>> +};
> >>> +module_acpi_driver(cros_usbpd_notify_acpi_driver);
> >>> +
> >>> +#endif /* CONFIG_ACPI */
> >>> +
> >>> +#ifdef CONFIG_OF
> >>> +
> >>> +static int cros_usbpd_notify_plat(struct notifier_block *nb,
> >>> +             unsigned long queued_during_suspend, void *data)
> >>> +{
> >>> +     struct cros_ec_device *ec_dev =3D (struct cros_ec_device *)data=
;
> >>> +     u32 host_event =3D cros_ec_get_host_event(ec_dev);
> >>> +
> >>> +     if (!host_event)
> >>> +             return NOTIFY_BAD;
> >>> +
> >>> +     if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
> >>> +             blocking_notifier_call_chain(&cros_usbpd_notifier_list,
> >>> +                             host_event, NULL);
> >>> +             return NOTIFY_OK;
> >>> +     }
> >>> +     return NOTIFY_DONE;
> >>> +}
> >>> +
> >>> +static int cros_usbpd_notify_probe_plat(struct platform_device *pdev=
)
> >>> +{
> >>> +     struct device *dev =3D &pdev->dev;
> >>> +     struct cros_ec_dev *ecdev =3D dev_get_drvdata(dev->parent);
> >>> +     struct notifier_block *nb;
> >>> +     int ret;
> >>> +
> >>> +     nb =3D devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
> >>> +     if (!nb)
> >>> +             return -ENOMEM;
> >>> +
> >>> +     nb->notifier_call =3D cros_usbpd_notify_plat;
> >>> +     dev_set_drvdata(dev, nb);
> >>> +
> >>> +     ret =3D blocking_notifier_chain_register(&ecdev->ec_dev->event_=
notifier,
> >>> +                                             nb);
> >>> +     if (ret < 0) {
> >>> +             dev_err(dev, "Failed to register notifier\n");
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int cros_usbpd_notify_remove_plat(struct platform_device *pde=
v)
> >>> +{
> >>> +     struct device *dev =3D &pdev->dev;
> >>> +     struct cros_ec_dev *ecdev =3D dev_get_drvdata(dev->parent);
> >>> +     struct notifier_block *nb =3D
> >>> +             (struct notifier_block *)dev_get_drvdata(dev);
> >>> +
> >>> +     blocking_notifier_chain_unregister(&ecdev->ec_dev->event_notifi=
er,
> >>> +                     nb);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static struct platform_driver cros_usbpd_notify_plat_driver =3D {
> >>> +     .driver =3D {
> >>> +             .name =3D DRV_NAME,
> >>> +     },
> >>> +     .probe =3D cros_usbpd_notify_probe_plat,
> >>> +     .remove =3D cros_usbpd_notify_remove_plat,
> >>> +};
> >>> +module_platform_driver(cros_usbpd_notify_plat_driver);
> >>> +
> >>> +#endif /* CONFIG_OF */
> >>> +
> >>> +MODULE_LICENSE("GPL");
> >>> +MODULE_DESCRIPTION("ChromeOS power delivery notifier device");
> >>> +MODULE_AUTHOR("Jon Flatley <jflat@chromium.org>");
> >>> +MODULE_ALIAS("platform:" DRV_NAME);
> >>> diff --git a/include/linux/platform_data/cros_usbpd_notify.h b/includ=
e/linux/platform_data/cros_usbpd_notify.h
> >>> new file mode 100644
> >>> index 0000000000000..4f2791722b6d3
> >>> --- /dev/null
> >>> +++ b/include/linux/platform_data/cros_usbpd_notify.h
> >>> @@ -0,0 +1,17 @@
> >>> +// SPDX-License-Identifier: GPL-2.0-only
> >>> +/*
> >>> + * ChromeOS EC Power Delivery Notifier Driver
> >>> + *
> >>> + * Copyright 2020 Google LLC
> >>> + */
> >>> +
> >>> +#ifndef __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
> >>> +#define __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
> >>> +
> >>> +#include <linux/notifier.h>
> >>> +
> >>> +int cros_usbpd_register_notify(struct notifier_block *nb);
> >>> +
> >>> +void cros_usbpd_unregister_notify(struct notifier_block *nb);
> >>> +
> >>> +#endif  /* __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H */
> >>>