[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4ddf93f1-fe0c-763c-25d6-144bdce5cd4c@collabora.com>
Date: Tue, 14 Jan 2020 21:29:25 +0100
From: Enric Balletbo i Serra <enric.balletbo@...labora.com>
To: Prashant Malani <pmalani@...omium.org>
Cc: Guenter Roeck <groeck@...omium.org>,
Benson Leung <bleung@...omium.org>,
Lee Jones <lee.jones@...aro.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Jon Flatley <jflat@...omium.org>,
Gwendal Grignou <gwendal@...omium.org>
Subject: Re: [PATCH v5 1/2] platform: chrome: Add cros-usbpd-notify driver
On 14/1/20 20:23, Prashant Malani wrote:
> On Tue, Jan 14, 2020 at 9:07 AM Enric Balletbo i Serra
> <enric.balletbo@...labora.com> wrote:
>>
>> Hi Prashant,
>>
>> On 14/1/20 4:10, Prashant Malani wrote:
>>> From: Jon Flatley <jflat@...omium.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:
‘cros_usbpd_acpi_device_ids’ undeclared here (not in a function); did you mean
‘cros_usbpd_notify_acpi_device_ids’?
The others I didn't dig in. My build commands are the common.
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- defconfig
# Enable the new cros_usbpd_notify driver
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j4
Cheers,
Enric
>> Apart from this I'm getting build errors with my defconfig which has CONFIG_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 introduces
>> the user of this notifier. If you are only testing the ACPI case you can 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 = {
>>> + .name = DRV_NAME,
>>> + .class = DRV_NAME,
>>> + .ids = cros_usbpd_notify_acpi_device_ids,
>>> + .ops = {
>>> + .add = cros_usbpd_notify_add_acpi,
>>> + .notify = 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 = (struct cros_ec_device *)data;
>>> + u32 host_event = 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 = &pdev->dev;
>>> + struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
>>> + struct notifier_block *nb;
>>> + int ret;
>>> +
>>> + nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
>>> + if (!nb)
>>> + return -ENOMEM;
>>> +
>>> + nb->notifier_call = cros_usbpd_notify_plat;
>>> + dev_set_drvdata(dev, nb);
>>> +
>>> + ret = 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 *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
>>> + struct notifier_block *nb =
>>> + (struct notifier_block *)dev_get_drvdata(dev);
>>> +
>>> + blocking_notifier_chain_unregister(&ecdev->ec_dev->event_notifier,
>>> + nb);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct platform_driver cros_usbpd_notify_plat_driver = {
>>> + .driver = {
>>> + .name = DRV_NAME,
>>> + },
>>> + .probe = cros_usbpd_notify_probe_plat,
>>> + .remove = 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@...omium.org>");
>>> +MODULE_ALIAS("platform:" DRV_NAME);
>>> diff --git a/include/linux/platform_data/cros_usbpd_notify.h b/include/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 */
>>>
Powered by blists - more mailing lists