2021-12-22 16:21:02

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v2 1/5] i2c: Introduce common module to instantiate CCGx UCSI

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 <[email protected]>
---
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);
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_ */
--
2.34.1



2021-12-27 06:31:21

by Shah, Nehal-bakulchandra

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] i2c: Introduce common module to instantiate CCGx UCSI

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 <[email protected]>
> ---
> 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

2021-12-27 17:17:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] i2c: Introduce common module to instantiate CCGx UCSI

On Mon, Dec 27, 2021 at 6:45 PM Shah, Nehal-bakulchandra
<[email protected]> wrote:
> Thanks for the this patch series.
> On 12/22/2021 9:50 PM, Andy Shevchenko wrote:

...

> > +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.

Thanks!
Dunno if i need to resend or Wolfram can add it when applying.

...

> > +struct i2c_client *i2c_new_ccgx_ucsi(struct i2c_adapter *adapter, int irq,
> > + const struct software_node *swnode);

> 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.

We don't add dead code in the kernel, so when you have patches ready
just create another one as a prerequisite that adds that.

--
With Best Regards,
Andy Shevchenko