| Message ID | 20240528-yoga-ec-driver-v4-3-4fa8dfaae7b6@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4,1/6] dt-bindings: platform: Add Lenovo Yoga C630 EC | expand |
On Tue, 28 May 2024, Dmitry Baryshkov wrote: > The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in > the onboard EC. Add glue driver to interface the platform's UCSI > implementation. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/usb/typec/ucsi/Kconfig | 9 ++ > drivers/usb/typec/ucsi/Makefile | 1 + > drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ > 3 files changed, 199 insertions(+) > > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig > index bdcb1764cfae..680e1b87b152 100644 > --- a/drivers/usb/typec/ucsi/Kconfig > +++ b/drivers/usb/typec/ucsi/Kconfig > @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK > To compile the driver as a module, choose M here: the module will be > called ucsi_glink. > > +config UCSI_LENOVO_YOGA_C630 > + tristate "UCSI Interface Driver for Lenovo Yoga C630" > + depends on EC_LENOVO_YOGA_C630 > + help > + This driver enables UCSI support on the Lenovo Yoga C630 laptop. > + > + To compile the driver as a module, choose M here: the module will be > + called ucsi_yoga_c630. > + > endif > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > index b4679f94696b..aed41d23887b 100644 > --- a/drivers/usb/typec/ucsi/Makefile > +++ b/drivers/usb/typec/ucsi/Makefile > @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o > obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o > obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o > obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o > +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o > diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > new file mode 100644 > index 000000000000..ca1ab5c81b87 > --- /dev/null > +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > @@ -0,0 +1,189 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2024, Linaro Ltd > + * Authors: > + * Bjorn Andersson > + * Dmitry Baryshkov > + */ > +#include <linux/auxiliary_bus.h> > +#include <linux/module.h> > +#include <linux/platform_data/lenovo-yoga-c630.h> > + > +#include "ucsi.h" > + > +struct yoga_c630_ucsi { > + struct yoga_c630_ec *ec; > + struct ucsi *ucsi; > + struct notifier_block nb; > + struct completion complete; Add includes for what you used here. > + unsigned long flags; > +#define UCSI_C630_COMMAND_PENDING 0 > +#define UCSI_C630_ACK_PENDING 1 > + u16 version; > +}; > + > +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, extra space > + void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); Missing include for ucsi_get_drvdata > + u8 buf[YOGA_C630_UCSI_READ_SIZE]; > + int ret; > + > + ret = yoga_c630_ec_ucsi_read(uec->ec, buf); > + if (ret) > + return ret; > + > + if (offset == UCSI_VERSION) { > + memcpy(val, &uec->version, min(val_len, sizeof(uec->version))); > + return 0; > + } > + > + if (offset == UCSI_CCI) > + memcpy(val, buf, > + min(val_len, YOGA_C630_UCSI_CCI_SIZE)); Fits to one line. > + else if (offset == UCSI_MESSAGE_IN) > + memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE, > + min(val_len, YOGA_C630_UCSI_DATA_SIZE)); > + else > + return -EINVAL; > + > + return 0; > +} > + > +static int yoga_c630_ucsi_async_write(struct ucsi *ucsi, unsigned int offset, extra space, there seems to be more of them below but I won't mark them. > + const void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > + > + if (offset != UCSI_CONTROL || > + val_len != YOGA_C630_UCSI_WRITE_SIZE) > + return -EINVAL; > + > + return yoga_c630_ec_ucsi_write(uec->ec, val); > +} > + > +static int yoga_c630_ucsi_sync_write(struct ucsi *ucsi, unsigned int offset, > + const void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; > + int ret; > + > + if (ack) > + set_bit(UCSI_C630_ACK_PENDING, &uec->flags); > + else > + set_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); Include for set_bit() > + reinit_completion(&uec->complete); > + > + ret = yoga_c630_ucsi_async_write(ucsi, offset, val, val_len); > + if (ret) > + goto out_clear_bit; > + > + if (!wait_for_completion_timeout(&uec->complete, 5 * HZ)) > + ret = -ETIMEDOUT; > + > +out_clear_bit: > + if (ack) > + clear_bit(UCSI_C630_ACK_PENDING, &uec->flags); > + else > + clear_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > + > + return ret; > +} > + > +const struct ucsi_operations yoga_c630_ucsi_ops = { Include for ucsi_operations. > + .read = yoga_c630_ucsi_read, > + .sync_write = yoga_c630_ucsi_sync_write, > + .async_write = yoga_c630_ucsi_async_write, > +}; > + > +static int yoga_c630_ucsi_notify(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct yoga_c630_ucsi *uec = container_of(nb, struct yoga_c630_ucsi, nb); Include for container_of
On Wed, 29 May 2024 at 17:20, Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > On Tue, 28 May 2024, Dmitry Baryshkov wrote: > > > The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in > > the onboard EC. Add glue driver to interface the platform's UCSI > > implementation. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/usb/typec/ucsi/Kconfig | 9 ++ > > drivers/usb/typec/ucsi/Makefile | 1 + > > drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ > > 3 files changed, 199 insertions(+) > > > > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig > > index bdcb1764cfae..680e1b87b152 100644 > > --- a/drivers/usb/typec/ucsi/Kconfig > > +++ b/drivers/usb/typec/ucsi/Kconfig > > @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK > > To compile the driver as a module, choose M here: the module will be > > called ucsi_glink. > > > > +config UCSI_LENOVO_YOGA_C630 > > + tristate "UCSI Interface Driver for Lenovo Yoga C630" > > + depends on EC_LENOVO_YOGA_C630 > > + help > > + This driver enables UCSI support on the Lenovo Yoga C630 laptop. > > + > > + To compile the driver as a module, choose M here: the module will be > > + called ucsi_yoga_c630. > > + > > endif > > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > > index b4679f94696b..aed41d23887b 100644 > > --- a/drivers/usb/typec/ucsi/Makefile > > +++ b/drivers/usb/typec/ucsi/Makefile > > @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o > > obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o > > obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o > > obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o > > +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o > > diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > new file mode 100644 > > index 000000000000..ca1ab5c81b87 > > --- /dev/null > > +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > @@ -0,0 +1,189 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2022-2024, Linaro Ltd > > + * Authors: > > + * Bjorn Andersson > > + * Dmitry Baryshkov > > + */ > > +#include <linux/auxiliary_bus.h> > > +#include <linux/module.h> > > +#include <linux/platform_data/lenovo-yoga-c630.h> > > + > > +#include "ucsi.h" > > + > > +struct yoga_c630_ucsi { > > + struct yoga_c630_ec *ec; > > + struct ucsi *ucsi; > > + struct notifier_block nb; > > + struct completion complete; > > Add includes for what you used here. > > > + unsigned long flags; > > +#define UCSI_C630_COMMAND_PENDING 0 > > +#define UCSI_C630_ACK_PENDING 1 > > + u16 version; > > +}; > > + > > +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, > > extra space > > > + void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > Missing include for ucsi_get_drvdata I'll review my includes, but this comment and the comment for ucsi_operations are clearly wrong. There is a corresponding include. > > > + u8 buf[YOGA_C630_UCSI_READ_SIZE]; > > + int ret; > > + > > + ret = yoga_c630_ec_ucsi_read(uec->ec, buf); > > + if (ret) > > + return ret; > > + > > + if (offset == UCSI_VERSION) { > > + memcpy(val, &uec->version, min(val_len, sizeof(uec->version))); > > + return 0; > > + } > > + > > + if (offset == UCSI_CCI) > > + memcpy(val, buf, > > + min(val_len, YOGA_C630_UCSI_CCI_SIZE)); > > Fits to one line. > > > + else if (offset == UCSI_MESSAGE_IN) > > + memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE, > > + min(val_len, YOGA_C630_UCSI_DATA_SIZE)); > > + else > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static int yoga_c630_ucsi_async_write(struct ucsi *ucsi, unsigned int offset, > > extra space, there seems to be more of them below but I won't mark them. > > > + const void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > + > > + if (offset != UCSI_CONTROL || > > + val_len != YOGA_C630_UCSI_WRITE_SIZE) > > + return -EINVAL; > > + > > + return yoga_c630_ec_ucsi_write(uec->ec, val); > > +} > > + > > +static int yoga_c630_ucsi_sync_write(struct ucsi *ucsi, unsigned int offset, > > + const void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; > > + int ret; > > + > > + if (ack) > > + set_bit(UCSI_C630_ACK_PENDING, &uec->flags); > > + else > > + set_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > > Include for set_bit() > > > + reinit_completion(&uec->complete); > > + > > + ret = yoga_c630_ucsi_async_write(ucsi, offset, val, val_len); > > + if (ret) > > + goto out_clear_bit; > > + > > + if (!wait_for_completion_timeout(&uec->complete, 5 * HZ)) > > + ret = -ETIMEDOUT; > > + > > +out_clear_bit: > > + if (ack) > > + clear_bit(UCSI_C630_ACK_PENDING, &uec->flags); > > + else > > + clear_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > > + > > + return ret; > > +} > > + > > +const struct ucsi_operations yoga_c630_ucsi_ops = { > > Include for ucsi_operations. > > > + .read = yoga_c630_ucsi_read, > > + .sync_write = yoga_c630_ucsi_sync_write, > > + .async_write = yoga_c630_ucsi_async_write, > > +}; > > + > > +static int yoga_c630_ucsi_notify(struct notifier_block *nb, > > + unsigned long action, void *data) > > +{ > > + struct yoga_c630_ucsi *uec = container_of(nb, struct yoga_c630_ucsi, nb); > > Include for container_of > > -- > i. >
On Wed, 29 May 2024, Dmitry Baryshkov wrote: > On Wed, 29 May 2024 at 17:20, Ilpo Järvinen > <ilpo.jarvinen@linux.intel.com> wrote: > > > > On Tue, 28 May 2024, Dmitry Baryshkov wrote: > > > > > The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in > > > the onboard EC. Add glue driver to interface the platform's UCSI > > > implementation. > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > > --- > > > drivers/usb/typec/ucsi/Kconfig | 9 ++ > > > drivers/usb/typec/ucsi/Makefile | 1 + > > > drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ > > > 3 files changed, 199 insertions(+) > > > > > > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig > > > index bdcb1764cfae..680e1b87b152 100644 > > > --- a/drivers/usb/typec/ucsi/Kconfig > > > +++ b/drivers/usb/typec/ucsi/Kconfig > > > @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK > > > To compile the driver as a module, choose M here: the module will be > > > called ucsi_glink. > > > > > > +config UCSI_LENOVO_YOGA_C630 > > > + tristate "UCSI Interface Driver for Lenovo Yoga C630" > > > + depends on EC_LENOVO_YOGA_C630 > > > + help > > > + This driver enables UCSI support on the Lenovo Yoga C630 laptop. > > > + > > > + To compile the driver as a module, choose M here: the module will be > > > + called ucsi_yoga_c630. > > > + > > > endif > > > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > > > index b4679f94696b..aed41d23887b 100644 > > > --- a/drivers/usb/typec/ucsi/Makefile > > > +++ b/drivers/usb/typec/ucsi/Makefile > > > @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o > > > obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o > > > obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o > > > obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o > > > +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o > > > diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > > new file mode 100644 > > > index 000000000000..ca1ab5c81b87 > > > --- /dev/null > > > +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > > @@ -0,0 +1,189 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (c) 2022-2024, Linaro Ltd > > > + * Authors: > > > + * Bjorn Andersson > > > + * Dmitry Baryshkov > > > + */ > > > +#include <linux/auxiliary_bus.h> > > > +#include <linux/module.h> > > > +#include <linux/platform_data/lenovo-yoga-c630.h> > > > + > > > +#include "ucsi.h" > > > + > > > +struct yoga_c630_ucsi { > > > + struct yoga_c630_ec *ec; > > > + struct ucsi *ucsi; > > > + struct notifier_block nb; > > > + struct completion complete; > > > > Add includes for what you used here. > > > > > + unsigned long flags; > > > +#define UCSI_C630_COMMAND_PENDING 0 > > > +#define UCSI_C630_ACK_PENDING 1 > > > + u16 version; > > > +}; > > > + > > > +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, > > > > extra space > > > > > + void *val, size_t val_len) > > > +{ > > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > > > Missing include for ucsi_get_drvdata > > I'll review my includes, but this comment and the comment for > ucsi_operations are clearly wrong. There is a corresponding include. Ah, sorry about that. I completely missed there was that local include.
On 28/05/2024 21:44, Dmitry Baryshkov wrote: > The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in > the onboard EC. Add glue driver to interface the platform's UCSI > implementation. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/usb/typec/ucsi/Kconfig | 9 ++ > drivers/usb/typec/ucsi/Makefile | 1 + > drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ > 3 files changed, 199 insertions(+) > > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig > index bdcb1764cfae..680e1b87b152 100644 > --- a/drivers/usb/typec/ucsi/Kconfig > +++ b/drivers/usb/typec/ucsi/Kconfig > @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK > To compile the driver as a module, choose M here: the module will be > called ucsi_glink. > > +config UCSI_LENOVO_YOGA_C630 > + tristate "UCSI Interface Driver for Lenovo Yoga C630" > + depends on EC_LENOVO_YOGA_C630 > + help > + This driver enables UCSI support on the Lenovo Yoga C630 laptop. > + > + To compile the driver as a module, choose M here: the module will be > + called ucsi_yoga_c630. > + > endif > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > index b4679f94696b..aed41d23887b 100644 > --- a/drivers/usb/typec/ucsi/Makefile > +++ b/drivers/usb/typec/ucsi/Makefile > @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o > obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o > obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o > obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o > +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o > diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > new file mode 100644 > index 000000000000..ca1ab5c81b87 > --- /dev/null > +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > @@ -0,0 +1,189 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2024, Linaro Ltd > + * Authors: > + * Bjorn Andersson > + * Dmitry Baryshkov > + */ > +#include <linux/auxiliary_bus.h> > +#include <linux/module.h> > +#include <linux/platform_data/lenovo-yoga-c630.h> > + > +#include "ucsi.h" > + > +struct yoga_c630_ucsi { > + struct yoga_c630_ec *ec; > + struct ucsi *ucsi; > + struct notifier_block nb; > + struct completion complete; > + unsigned long flags; > +#define UCSI_C630_COMMAND_PENDING 0 > +#define UCSI_C630_ACK_PENDING 1 > + u16 version; > +}; > + > +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, > + void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > + u8 buf[YOGA_C630_UCSI_READ_SIZE]; > + int ret; > + > + ret = yoga_c630_ec_ucsi_read(uec->ec, buf); > + if (ret) > + return ret; > + > + if (offset == UCSI_VERSION) { > + memcpy(val, &uec->version, min(val_len, sizeof(uec->version))); > + return 0; > + } > + > + if (offset == UCSI_CCI) > + memcpy(val, buf, > + min(val_len, YOGA_C630_UCSI_CCI_SIZE)); > + else if (offset == UCSI_MESSAGE_IN) > + memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE, > + min(val_len, YOGA_C630_UCSI_DATA_SIZE)); For some reason I believe multi-lines like this, including function calls that are split over lines should be encapsulated with {} else if(x) { memcpy(x,y, z); } If checkpatch doesn't complain about it feel free not to do that though. > + else > + return -EINVAL; > + > + return 0; > +} > + > +static int yoga_c630_ucsi_async_write(struct ucsi *ucsi, unsigned int offset, > + const void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > + > + if (offset != UCSI_CONTROL || > + val_len != YOGA_C630_UCSI_WRITE_SIZE) > + return -EINVAL; > + > + return yoga_c630_ec_ucsi_write(uec->ec, val); > +} > + > +static int yoga_c630_ucsi_sync_write(struct ucsi *ucsi, unsigned int offset, > + const void *val, size_t val_len) > +{ > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; > + int ret; > + > + if (ack) > + set_bit(UCSI_C630_ACK_PENDING, &uec->flags); > + else > + set_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > + > + reinit_completion(&uec->complete); > + > + ret = yoga_c630_ucsi_async_write(ucsi, offset, val, val_len); > + if (ret) > + goto out_clear_bit; > + > + if (!wait_for_completion_timeout(&uec->complete, 5 * HZ)) > + ret = -ETIMEDOUT; > + > +out_clear_bit: > + if (ack) > + clear_bit(UCSI_C630_ACK_PENDING, &uec->flags); > + else > + clear_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > + > + return ret; > +} > + > +const struct ucsi_operations yoga_c630_ucsi_ops = { > + .read = yoga_c630_ucsi_read, > + .sync_write = yoga_c630_ucsi_sync_write, > + .async_write = yoga_c630_ucsi_async_write, > +}; > + > +static int yoga_c630_ucsi_notify(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct yoga_c630_ucsi *uec = container_of(nb, struct yoga_c630_ucsi, nb); > + u32 cci; > + int ret; > + > + if (action == LENOVO_EC_EVENT_USB || action == LENOVO_EC_EVENT_HPD) { > + ucsi_connector_change(uec->ucsi, 1); > + return NOTIFY_OK; > + } > + > + if (action != LENOVO_EC_EVENT_UCSI) > + return NOTIFY_DONE; Is this disjunction on action a good candidate for a switch(){} > + > + ret = uec->ucsi->ops->read(uec->ucsi, UCSI_CCI, &cci, sizeof(cci)); > + if (ret) > + return NOTIFY_DONE; > + > + if (UCSI_CCI_CONNECTOR(cci)) > + ucsi_connector_change(uec->ucsi, UCSI_CCI_CONNECTOR(cci)); > + > + if (cci & UCSI_CCI_ACK_COMPLETE && > + test_bit(UCSI_C630_ACK_PENDING, &uec->flags)) > + complete(&uec->complete); > + if (cci & UCSI_CCI_COMMAND_COMPLETE && > + test_bit(UCSI_C630_COMMAND_PENDING, &uec->flags)) > + complete(&uec->complete); IMO these multi-line clauses should end up with a {} around the complete even though its not required. Emphasis on the O. Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On Wed, May 29, 2024 at 04:41:40PM +0100, Bryan O'Donoghue wrote: > On 28/05/2024 21:44, Dmitry Baryshkov wrote: > > The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in > > the onboard EC. Add glue driver to interface the platform's UCSI > > implementation. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/usb/typec/ucsi/Kconfig | 9 ++ > > drivers/usb/typec/ucsi/Makefile | 1 + > > drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ > > 3 files changed, 199 insertions(+) > > > > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig > > index bdcb1764cfae..680e1b87b152 100644 > > --- a/drivers/usb/typec/ucsi/Kconfig > > +++ b/drivers/usb/typec/ucsi/Kconfig > > @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK > > To compile the driver as a module, choose M here: the module will be > > called ucsi_glink. > > +config UCSI_LENOVO_YOGA_C630 > > + tristate "UCSI Interface Driver for Lenovo Yoga C630" > > + depends on EC_LENOVO_YOGA_C630 > > + help > > + This driver enables UCSI support on the Lenovo Yoga C630 laptop. > > + > > + To compile the driver as a module, choose M here: the module will be > > + called ucsi_yoga_c630. > > + > > endif > > diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile > > index b4679f94696b..aed41d23887b 100644 > > --- a/drivers/usb/typec/ucsi/Makefile > > +++ b/drivers/usb/typec/ucsi/Makefile > > @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o > > obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o > > obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o > > obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o > > +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o > > diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > new file mode 100644 > > index 000000000000..ca1ab5c81b87 > > --- /dev/null > > +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c > > @@ -0,0 +1,189 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2022-2024, Linaro Ltd > > + * Authors: > > + * Bjorn Andersson > > + * Dmitry Baryshkov > > + */ > > +#include <linux/auxiliary_bus.h> > > +#include <linux/module.h> > > +#include <linux/platform_data/lenovo-yoga-c630.h> > > + > > +#include "ucsi.h" > > + > > +struct yoga_c630_ucsi { > > + struct yoga_c630_ec *ec; > > + struct ucsi *ucsi; > > + struct notifier_block nb; > > + struct completion complete; > > + unsigned long flags; > > +#define UCSI_C630_COMMAND_PENDING 0 > > +#define UCSI_C630_ACK_PENDING 1 > > + u16 version; > > +}; > > + > > +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, > > + void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > + u8 buf[YOGA_C630_UCSI_READ_SIZE]; > > + int ret; > > + > > + ret = yoga_c630_ec_ucsi_read(uec->ec, buf); > > + if (ret) > > + return ret; > > + > > + if (offset == UCSI_VERSION) { > > + memcpy(val, &uec->version, min(val_len, sizeof(uec->version))); > > + return 0; > > + } > > + > > + if (offset == UCSI_CCI) > > + memcpy(val, buf, > > + min(val_len, YOGA_C630_UCSI_CCI_SIZE)); > > + else if (offset == UCSI_MESSAGE_IN) > > + memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE, > > + min(val_len, YOGA_C630_UCSI_DATA_SIZE)); > > For some reason I believe multi-lines like this, including function calls > that are split over lines should be encapsulated with {} > > else if(x) { > memcpy(x,y, > z); > } > > If checkpatch doesn't complain about it feel free not to do that though. No, checkpatch --strict doesn't complain > > > + else > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static int yoga_c630_ucsi_async_write(struct ucsi *ucsi, unsigned int offset, > > + const void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > + > > + if (offset != UCSI_CONTROL || > > + val_len != YOGA_C630_UCSI_WRITE_SIZE) > > + return -EINVAL; > > + > > + return yoga_c630_ec_ucsi_write(uec->ec, val); > > +} > > + > > +static int yoga_c630_ucsi_sync_write(struct ucsi *ucsi, unsigned int offset, > > + const void *val, size_t val_len) > > +{ > > + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); > > + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; > > + int ret; > > + > > + if (ack) > > + set_bit(UCSI_C630_ACK_PENDING, &uec->flags); > > + else > > + set_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > > + > > + reinit_completion(&uec->complete); > > + > > + ret = yoga_c630_ucsi_async_write(ucsi, offset, val, val_len); > > + if (ret) > > + goto out_clear_bit; > > + > > + if (!wait_for_completion_timeout(&uec->complete, 5 * HZ)) > > + ret = -ETIMEDOUT; > > + > > +out_clear_bit: > > + if (ack) > > + clear_bit(UCSI_C630_ACK_PENDING, &uec->flags); > > + else > > + clear_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); > > + > > + return ret; > > +} > > + > > +const struct ucsi_operations yoga_c630_ucsi_ops = { > > + .read = yoga_c630_ucsi_read, > > + .sync_write = yoga_c630_ucsi_sync_write, > > + .async_write = yoga_c630_ucsi_async_write, > > +}; > > + > > +static int yoga_c630_ucsi_notify(struct notifier_block *nb, > > + unsigned long action, void *data) > > +{ > > + struct yoga_c630_ucsi *uec = container_of(nb, struct yoga_c630_ucsi, nb); > > + u32 cci; > > + int ret; > > + > > + if (action == LENOVO_EC_EVENT_USB || action == LENOVO_EC_EVENT_HPD) { > > + ucsi_connector_change(uec->ucsi, 1); > > + return NOTIFY_OK; > > + } > > + > > + if (action != LENOVO_EC_EVENT_UCSI) > > + return NOTIFY_DONE; > > Is this disjunction on action a good candidate for a switch(){} Ack, refactored the function by extracting the UCSI notification code and then using the switch-case. > > + > > + ret = uec->ucsi->ops->read(uec->ucsi, UCSI_CCI, &cci, sizeof(cci)); > > + if (ret) > > + return NOTIFY_DONE; > > + > > + if (UCSI_CCI_CONNECTOR(cci)) > > + ucsi_connector_change(uec->ucsi, UCSI_CCI_CONNECTOR(cci)); > > + > > + if (cci & UCSI_CCI_ACK_COMPLETE && > > + test_bit(UCSI_C630_ACK_PENDING, &uec->flags)) > > + complete(&uec->complete); > > + if (cci & UCSI_CCI_COMMAND_COMPLETE && > > + test_bit(UCSI_C630_COMMAND_PENDING, &uec->flags)) > > + complete(&uec->complete); > > IMO these multi-line clauses should end up with a {} around the complete > even though its not required. > > Emphasis on the O. I added an empty line inbetween, then it's easier to comprehent event without curly brackets. > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig index bdcb1764cfae..680e1b87b152 100644 --- a/drivers/usb/typec/ucsi/Kconfig +++ b/drivers/usb/typec/ucsi/Kconfig @@ -69,4 +69,13 @@ config UCSI_PMIC_GLINK To compile the driver as a module, choose M here: the module will be called ucsi_glink. +config UCSI_LENOVO_YOGA_C630 + tristate "UCSI Interface Driver for Lenovo Yoga C630" + depends on EC_LENOVO_YOGA_C630 + help + This driver enables UCSI support on the Lenovo Yoga C630 laptop. + + To compile the driver as a module, choose M here: the module will be + called ucsi_yoga_c630. + endif diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile index b4679f94696b..aed41d23887b 100644 --- a/drivers/usb/typec/ucsi/Makefile +++ b/drivers/usb/typec/ucsi/Makefile @@ -21,3 +21,4 @@ obj-$(CONFIG_UCSI_ACPI) += ucsi_acpi.o obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o obj-$(CONFIG_UCSI_STM32G0) += ucsi_stm32g0.o obj-$(CONFIG_UCSI_PMIC_GLINK) += ucsi_glink.o +obj-$(CONFIG_UCSI_LENOVO_YOGA_C630) += ucsi_yoga_c630.o diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c new file mode 100644 index 000000000000..ca1ab5c81b87 --- /dev/null +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c @@ -0,0 +1,189 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2022-2024, Linaro Ltd + * Authors: + * Bjorn Andersson + * Dmitry Baryshkov + */ +#include <linux/auxiliary_bus.h> +#include <linux/module.h> +#include <linux/platform_data/lenovo-yoga-c630.h> + +#include "ucsi.h" + +struct yoga_c630_ucsi { + struct yoga_c630_ec *ec; + struct ucsi *ucsi; + struct notifier_block nb; + struct completion complete; + unsigned long flags; +#define UCSI_C630_COMMAND_PENDING 0 +#define UCSI_C630_ACK_PENDING 1 + u16 version; +}; + +static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset, + void *val, size_t val_len) +{ + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); + u8 buf[YOGA_C630_UCSI_READ_SIZE]; + int ret; + + ret = yoga_c630_ec_ucsi_read(uec->ec, buf); + if (ret) + return ret; + + if (offset == UCSI_VERSION) { + memcpy(val, &uec->version, min(val_len, sizeof(uec->version))); + return 0; + } + + if (offset == UCSI_CCI) + memcpy(val, buf, + min(val_len, YOGA_C630_UCSI_CCI_SIZE)); + else if (offset == UCSI_MESSAGE_IN) + memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE, + min(val_len, YOGA_C630_UCSI_DATA_SIZE)); + else + return -EINVAL; + + return 0; +} + +static int yoga_c630_ucsi_async_write(struct ucsi *ucsi, unsigned int offset, + const void *val, size_t val_len) +{ + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); + + if (offset != UCSI_CONTROL || + val_len != YOGA_C630_UCSI_WRITE_SIZE) + return -EINVAL; + + return yoga_c630_ec_ucsi_write(uec->ec, val); +} + +static int yoga_c630_ucsi_sync_write(struct ucsi *ucsi, unsigned int offset, + const void *val, size_t val_len) +{ + struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi); + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI; + int ret; + + if (ack) + set_bit(UCSI_C630_ACK_PENDING, &uec->flags); + else + set_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); + + reinit_completion(&uec->complete); + + ret = yoga_c630_ucsi_async_write(ucsi, offset, val, val_len); + if (ret) + goto out_clear_bit; + + if (!wait_for_completion_timeout(&uec->complete, 5 * HZ)) + ret = -ETIMEDOUT; + +out_clear_bit: + if (ack) + clear_bit(UCSI_C630_ACK_PENDING, &uec->flags); + else + clear_bit(UCSI_C630_COMMAND_PENDING, &uec->flags); + + return ret; +} + +const struct ucsi_operations yoga_c630_ucsi_ops = { + .read = yoga_c630_ucsi_read, + .sync_write = yoga_c630_ucsi_sync_write, + .async_write = yoga_c630_ucsi_async_write, +}; + +static int yoga_c630_ucsi_notify(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct yoga_c630_ucsi *uec = container_of(nb, struct yoga_c630_ucsi, nb); + u32 cci; + int ret; + + if (action == LENOVO_EC_EVENT_USB || action == LENOVO_EC_EVENT_HPD) { + ucsi_connector_change(uec->ucsi, 1); + return NOTIFY_OK; + } + + if (action != LENOVO_EC_EVENT_UCSI) + return NOTIFY_DONE; + + ret = uec->ucsi->ops->read(uec->ucsi, UCSI_CCI, &cci, sizeof(cci)); + if (ret) + return NOTIFY_DONE; + + if (UCSI_CCI_CONNECTOR(cci)) + ucsi_connector_change(uec->ucsi, UCSI_CCI_CONNECTOR(cci)); + + if (cci & UCSI_CCI_ACK_COMPLETE && + test_bit(UCSI_C630_ACK_PENDING, &uec->flags)) + complete(&uec->complete); + if (cci & UCSI_CCI_COMMAND_COMPLETE && + test_bit(UCSI_C630_COMMAND_PENDING, &uec->flags)) + complete(&uec->complete); + + return NOTIFY_OK; +} + +static int yoga_c630_ucsi_probe(struct auxiliary_device *adev, + const struct auxiliary_device_id *id) +{ + struct yoga_c630_ec *ec = adev->dev.platform_data; + struct yoga_c630_ucsi *uec; + int ret; + + uec = devm_kzalloc(&adev->dev, sizeof(*uec), GFP_KERNEL); + if (!uec) + return -ENOMEM; + + uec->ec = ec; + init_completion(&uec->complete); + uec->nb.notifier_call = yoga_c630_ucsi_notify; + + uec->ucsi = ucsi_create(&adev->dev, &yoga_c630_ucsi_ops); + if (IS_ERR(uec->ucsi)) + return PTR_ERR(uec->ucsi); + + ucsi_set_drvdata(uec->ucsi, uec); + + uec->version = yoga_c630_ec_ucsi_get_version(uec->ec); + + auxiliary_set_drvdata(adev, uec); + + ret = yoga_c630_ec_register_notify(ec, &uec->nb); + if (ret) + return ret; + + return ucsi_register(uec->ucsi); +} + +static void yoga_c630_ucsi_remove(struct auxiliary_device *adev) +{ + struct yoga_c630_ucsi *uec = auxiliary_get_drvdata(adev); + + yoga_c630_ec_unregister_notify(uec->ec, &uec->nb); + ucsi_unregister(uec->ucsi); +} + +static const struct auxiliary_device_id yoga_c630_ucsi_id_table[] = { + { .name = YOGA_C630_MOD_NAME "." YOGA_C630_DEV_UCSI, }, + {} +}; +MODULE_DEVICE_TABLE(auxiliary, yoga_c630_ucsi_id_table); + +static struct auxiliary_driver yoga_c630_ucsi_driver = { + .name = YOGA_C630_DEV_UCSI, + .id_table = yoga_c630_ucsi_id_table, + .probe = yoga_c630_ucsi_probe, + .remove = yoga_c630_ucsi_remove, +}; + +module_auxiliary_driver(yoga_c630_ucsi_driver); + +MODULE_DESCRIPTION("Lenovo Yoga C630 UCSI"); +MODULE_LICENSE("GPL");
The Lenovo Yoga C630 WOS laptop provides implements UCSI interface in the onboard EC. Add glue driver to interface the platform's UCSI implementation. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/usb/typec/ucsi/Kconfig | 9 ++ drivers/usb/typec/ucsi/Makefile | 1 + drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 189 ++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+)