Messages in this thread | | | Date | Mon, 27 Dec 2021 12:01:00 +0530 | Subject | Re: [PATCH v2 1/5] i2c: Introduce common module to instantiate CCGx UCSI | From | "Shah, Nehal-bakulchandra" <> |
| |
Hi Andy
Thanks for the this patch series. On 12/22/2021 9:50 PM, Andy Shevchenko wrote: > Introduce a common module to provide an API to instantiate UCSI device > for Cypress CCGx Type-C controller. Individual bus drivers need to select > this one on demand. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: rebased on top of current i2c/for-next > drivers/i2c/busses/Kconfig | 7 +++++++ > drivers/i2c/busses/Makefile | 3 +++ > drivers/i2c/busses/i2c-ccgx-ucsi.c | 27 +++++++++++++++++++++++++++ > drivers/i2c/busses/i2c-ccgx-ucsi.h | 11 +++++++++++ > 4 files changed, 48 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.c > create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.h > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 42da31c1ab70..08e24e396e37 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -9,6 +9,13 @@ menu "I2C Hardware Bus support" > comment "PC SMBus host controller drivers" > depends on PCI > > +config I2C_CCGX_UCSI > + tristate > + help > + A common module to provide an API to instantiate UCSI device > + for Cypress CCGx Type-C controller. Individual bus drivers > + need to select this one on demand. > + > config I2C_ALI1535 > tristate "ALI 1535" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 1d00dce77098..79405cb5d600 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -6,6 +6,9 @@ > # ACPI drivers > obj-$(CONFIG_I2C_SCMI) += i2c-scmi.o > > +# Auxiliary I2C/SMBus modules > +obj-$(CONFIG_I2C_CCGX_UCSI) += i2c-ccgx-ucsi.o > + > # PC SMBus host controller drivers > obj-$(CONFIG_I2C_ALI1535) += i2c-ali1535.o > obj-$(CONFIG_I2C_ALI1563) += i2c-ali1563.o > diff --git a/drivers/i2c/busses/i2c-ccgx-ucsi.c b/drivers/i2c/busses/i2c-ccgx-ucsi.c > new file mode 100644 > index 000000000000..141c3d1ef752 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-ccgx-ucsi.c > @@ -0,0 +1,27 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Instantiate UCSI device for Cypress CCGx Type-C controller. > + * Derived from i2c-designware-pcidrv.c and i2c-nvidia-gpu.c. > + */ > + > +#include <linux/i2c.h> > +#include <linux/export.h> > +#include <linux/string.h> > + > +#include "i2c-ccgx-ucsi.h" > + > +struct software_node; > + > +struct i2c_client *i2c_new_ccgx_ucsi(struct i2c_adapter *adapter, int irq, > + const struct software_node *swnode) > +{ > + struct i2c_board_info info = {}; > + > + strscpy(info.type, "ccgx-ucsi", sizeof(info.type)); > + info.addr = 0x08; > + info.irq = irq; > + info.swnode = swnode; > + > + return i2c_new_client_device(adapter, &info); > +} > +EXPORT_SYMBOL_GPL(i2c_new_ccgx_ucsi);c it needs MODULE_LICENSE("GPL"); else if driver is built as module it fails to probe. However after adding this we validated and it is working fine. > diff --git a/drivers/i2c/busses/i2c-ccgx-ucsi.h b/drivers/i2c/busses/i2c-ccgx-ucsi.h > new file mode 100644 > index 000000000000..739ac7a4b117 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-ccgx-ucsi.h > @@ -0,0 +1,11 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +#ifndef __I2C_CCGX_UCSI_H_ > +#define __I2C_CCGX_UCSI_H_ > + > +struct i2c_adapter; > +struct i2c_client; > +struct software_node; > + > +struct i2c_client *i2c_new_ccgx_ucsi(struct i2c_adapter *adapter, int irq, > + const struct software_node *swnode); > +#endif /* __I2C_CCGX_UCSI_H_ */ >
Here, One more suggestion if can be incorporated , instead of passing only irq we should pass irq number and irq type. For example in our next generation platform , CCGX driver is using IRQF_TRIGGER_FALLING type where is default hard coded is IRQF_TRIGGER_HIGH. So in CCGX driver in request_threaded_irq function along with passing irq number , irq type also can be passed.
Regards Nehal Shah
| |