* [PATCH v2 0/4] Define i2c_designware in a header file
@ 2024-04-25 0:26 Florian Fainelli
2024-04-25 0:26 ` [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name Florian Fainelli
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Florian Fainelli @ 2024-04-25 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: Florian Fainelli, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maciej Fijalkowski, Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
This patch series depends upon the following two patches being applied:
https://lore.kernel.org/all/20240422084109.3201-1-duanqiangwen@net-swift.com/
https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@net-swift.com/
There is no reason why each driver should have to repeat the
"i2c_designware" string all over the place, because when that happens we
see the reverts like the above being necessary.
Changes in v2:
- avoid changing i2c-designware-pcidrv.c more than necessary
- move constant to include/linux/platform_data/i2c-designware.h
- add comments as to how this constant is used and why
Florian Fainelli (4):
i2c: designware: Create shared header hosting driver name
mfd: intel-lpss: Utilize i2c-designware.h
mfd: intel_quark_i2c_gpio: Utilize i2c-designware.h
net: txgbe: Utilize i2c-designware.h
MAINTAINERS | 1 +
drivers/i2c/busses/i2c-designware-pcidrv.c | 3 ++-
drivers/i2c/busses/i2c-designware-platdrv.c | 5 +++--
drivers/mfd/intel-lpss.c | 3 ++-
drivers/mfd/intel_quark_i2c_gpio.c | 5 +++--
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 7 ++++---
include/linux/platform_data/i2c-designware.h | 11 +++++++++++
7 files changed, 26 insertions(+), 9 deletions(-)
create mode 100644 include/linux/platform_data/i2c-designware.h
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
@ 2024-04-25 0:26 ` Florian Fainelli
2024-04-25 9:39 ` Andy Shevchenko
2024-04-25 0:26 ` [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h Florian Fainelli
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2024-04-25 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: Florian Fainelli, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maciej Fijalkowski, Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
We have a number of drivers that reference the string "i2c_designware"
towards two goals:
- ensure their device gets bound to the i2c_designware platform_driver
- create a clock lookup reference that matches the i2c_designware
instance number for the i2c-designware-platdrv.c driver to be able to
lookup the input reference clock
Since this string is copied in a bunch of different places and since it
is possible to get this named wrong (see [1] and [2]) with unintended
consequences, create a header file that hosts that define for other
drivers to use and all agree on the correct name to use.
[1]:
https://lore.kernel.org/all/20240422084109.3201-1-duanqiangwen@net-swift.com/
[2]:
https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@net-swift.com/
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
MAINTAINERS | 1 +
drivers/i2c/busses/i2c-designware-pcidrv.c | 3 ++-
drivers/i2c/busses/i2c-designware-platdrv.c | 5 +++--
include/linux/platform_data/i2c-designware.h | 11 +++++++++++
4 files changed, 17 insertions(+), 3 deletions(-)
create mode 100644 include/linux/platform_data/i2c-designware.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 2d5acd6d98c4..da1b39df2b94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21438,6 +21438,7 @@ R: Jan Dabros <jsd@semihalf.com>
L: linux-i2c@vger.kernel.org
S: Supported
F: drivers/i2c/busses/i2c-designware-*
+F: include/linux/platform_data/i2c-designware.h
SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER
M: Jaehoon Chung <jh80.chung@samsung.com>
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 9be9a2658e1f..32d6f338a1ea 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/pm_runtime.h>
#include <linux/power_supply.h>
#include <linux/sched.h>
@@ -425,7 +426,7 @@ static struct pci_driver dw_i2c_driver = {
module_pci_driver(dw_i2c_driver);
/* Work with hotplug and coldplug */
-MODULE_ALIAS("i2c_designware-pci");
+MODULE_ALIAS(I2C_DESIGNWARE_NAME "-pci");
MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>");
MODULE_DESCRIPTION("Synopsys DesignWare PCI I2C bus adapter");
MODULE_LICENSE("GPL");
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 4ab41ba39d55..8aa2bf274d78 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -22,6 +22,7 @@
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
@@ -480,13 +481,13 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
};
/* Work with hotplug and coldplug */
-MODULE_ALIAS("platform:i2c_designware");
+MODULE_ALIAS("platform:" I2C_DESIGNWARE_NAME);
static struct platform_driver dw_i2c_driver = {
.probe = dw_i2c_plat_probe,
.remove_new = dw_i2c_plat_remove,
.driver = {
- .name = "i2c_designware",
+ .name = I2C_DESIGNWARE_NAME,
.of_match_table = of_match_ptr(dw_i2c_of_match),
.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
.pm = pm_ptr(&dw_i2c_dev_pm_ops),
diff --git a/include/linux/platform_data/i2c-designware.h b/include/linux/platform_data/i2c-designware.h
new file mode 100644
index 000000000000..e385b6d70a04
--- /dev/null
+++ b/include/linux/platform_data/i2c-designware.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __I2C_DESIGNWARE_PDATA_H__
+#define __I2C_DESIGNWARE_PDATA_H__
+
+/* This is the i2c-designware-platdrv.c platform driver name. This name is used
+ * to bind the device to the driver, as well as by the driver itself to request
+ * the input reference clock
+ */
+#define I2C_DESIGNWARE_NAME "i2c_designware"
+
+#endif /* __I2C_DESIGNWARE_PDATA_H__ */
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
2024-04-25 0:26 ` [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name Florian Fainelli
@ 2024-04-25 0:26 ` Florian Fainelli
2024-04-25 9:34 ` Andy Shevchenko
2024-05-02 10:00 ` Lee Jones
2024-04-25 0:26 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: " Florian Fainelli
` (2 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Florian Fainelli @ 2024-04-25 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: Florian Fainelli, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maciej Fijalkowski, Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/mfd/intel-lpss.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 2a9018112dfc..551560beff7b 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -24,6 +24,7 @@
#include <linux/ioport.h>
#include <linux/mfd/core.h>
#include <linux/module.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/pm.h>
#include <linux/pm_qos.h>
#include <linux/pm_runtime.h>
@@ -116,7 +117,7 @@ static const struct mfd_cell intel_lpss_idma64_cell = {
};
static const struct mfd_cell intel_lpss_i2c_cell = {
- .name = "i2c_designware",
+ .name = I2C_DESIGNWARE_NAME,
.num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
.resources = intel_lpss_dev_resources,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Utilize i2c-designware.h
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
2024-04-25 0:26 ` [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name Florian Fainelli
2024-04-25 0:26 ` [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h Florian Fainelli
@ 2024-04-25 0:26 ` Florian Fainelli
2024-04-25 9:42 ` Andy Shevchenko
2024-04-25 0:26 ` [PATCH v2 4/4] net: txgbe: " Florian Fainelli
2024-04-25 13:11 ` [PATCH v2 0/4] Define i2c_designware in a header file Jarkko Nikula
4 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2024-04-25 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: Florian Fainelli, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maciej Fijalkowski, Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/mfd/intel_quark_i2c_gpio.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 9b9c76bd067b..08e296fc56f3 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -17,6 +17,7 @@
#include <linux/clk-provider.h>
#include <linux/dmi.h>
#include <linux/i2c.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/property.h>
/* PCI BAR for register base address */
@@ -30,7 +31,7 @@
#define INTEL_QUARK_IORES_MEM 0
#define INTEL_QUARK_IORES_IRQ 1
-#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
+#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
/* The Quark I2C controller source clock */
#define INTEL_QUARK_I2C_CLK_HZ 33000000
@@ -136,7 +137,7 @@ static const struct software_node *intel_quark_gpio_node_group[] = {
static struct mfd_cell intel_quark_mfd_cells[] = {
[MFD_I2C_BAR] = {
.id = MFD_I2C_BAR,
- .name = "i2c_designware",
+ .name = I2C_DESIGNWARE_NAME,
.acpi_match = &intel_quark_acpi_match_i2c,
.num_resources = ARRAY_SIZE(intel_quark_i2c_res),
.resources = intel_quark_i2c_res,
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 4/4] net: txgbe: Utilize i2c-designware.h
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
` (2 preceding siblings ...)
2024-04-25 0:26 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: " Florian Fainelli
@ 2024-04-25 0:26 ` Florian Fainelli
2024-04-25 9:44 ` Andy Shevchenko
2024-04-25 13:11 ` [PATCH v2 0/4] Define i2c_designware in a header file Jarkko Nikula
4 siblings, 1 reply; 11+ messages in thread
From: Florian Fainelli @ 2024-04-25 0:26 UTC (permalink / raw)
To: linux-kernel
Cc: Florian Fainelli, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Maciej Fijalkowski, Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 93295916b1d2..7545aacc9c19 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -8,6 +8,7 @@
#include <linux/clkdev.h>
#include <linux/i2c.h>
#include <linux/pci.h>
+#include <linux/platform_data/i2c-designware.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/pcs/pcs-xpcs.h>
@@ -571,8 +572,8 @@ static int txgbe_clock_register(struct txgbe *txgbe)
char clk_name[32];
struct clk *clk;
- snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
- pci_dev_id(pdev));
+ snprintf(clk_name, sizeof(clk_name), "%s.%d",
+ I2C_DESIGNWARE_NAME, pci_dev_id(pdev));
clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
if (IS_ERR(clk))
@@ -634,7 +635,7 @@ static int txgbe_i2c_register(struct txgbe *txgbe)
info.parent = &pdev->dev;
info.fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_I2C]);
- info.name = "i2c_designware";
+ info.name = I2C_DESIGNWARE_NAME;
info.id = pci_dev_id(pdev);
info.res = &DEFINE_RES_IRQ(pdev->irq);
--
2.34.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h
2024-04-25 0:26 ` [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h Florian Fainelli
@ 2024-04-25 9:34 ` Andy Shevchenko
2024-05-02 10:00 ` Lee Jones
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2024-04-25 9:34 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Jarkko Nikula, Mika Westerberg, Jan Dabros,
Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski,
Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On Wed, Apr 24, 2024 at 05:26:40PM -0700, Florian Fainelli wrote:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
...
> +#include <linux/platform_data/i2c-designware.h>
Please, group this with linux/dma/idma64.h below.
...
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
in case this go anywhere.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name
2024-04-25 0:26 ` [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name Florian Fainelli
@ 2024-04-25 9:39 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2024-04-25 9:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Jarkko Nikula, Mika Westerberg, Jan Dabros,
Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski,
Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On Wed, Apr 24, 2024 at 05:26:39PM -0700, Florian Fainelli wrote:
> We have a number of drivers that reference the string "i2c_designware"
> towards two goals:
>
> - ensure their device gets bound to the i2c_designware platform_driver
> - create a clock lookup reference that matches the i2c_designware
> instance number for the i2c-designware-platdrv.c driver to be able to
> lookup the input reference clock
>
> Since this string is copied in a bunch of different places and since it
> is possible to get this named wrong (see [1] and [2]) with unintended
> consequences, create a header file that hosts that define for other
> drivers to use and all agree on the correct name to use.
> [1]:
> https://lore.kernel.org/all/20240422084109.3201-1-duanqiangwen@net-swift.com/
> [2]:
> https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@net-swift.com/
Make them tags.
Link: URL#1 [1]
Link: URL#2 [2]
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -19,6 +19,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/platform_data/i2c-designware.h>
Please, make it a separate group after linux/* and before "xxx" below.
> #include <linux/pm_runtime.h>
> #include <linux/power_supply.h>
> #include <linux/sched.h>
...
> module_pci_driver(dw_i2c_driver);
>
> /* Work with hotplug and coldplug */
> -MODULE_ALIAS("i2c_designware-pci");
> +MODULE_ALIAS(I2C_DESIGNWARE_NAME "-pci");
As Jarkko said, please get rid of this.
You may take my patch from here:
https://lore.kernel.org/linux-i2c/20231207141653.2785124-9-andriy.shevchenko@linux.intel.com/
and incorporate in this series.
...
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/platform_data/i2c-designware.h>
As per above.
> #include <linux/platform_device.h>
> #include <linux/pm.h>
> #include <linux/pm_runtime.h>
...
> /* Work with hotplug and coldplug */
> -MODULE_ALIAS("platform:i2c_designware");
> +MODULE_ALIAS("platform:" I2C_DESIGNWARE_NAME);
See above.
...
> +/* This is the i2c-designware-platdrv.c platform driver name. This name is used
> + * to bind the device to the driver, as well as by the driver itself to request
> + * the input reference clock
> + */
/*
* Use correct multi-line comment style. This
* is not the net subsystem, we use traditional style.
*/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: Utilize i2c-designware.h
2024-04-25 0:26 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: " Florian Fainelli
@ 2024-04-25 9:42 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2024-04-25 9:42 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Jarkko Nikula, Mika Westerberg, Jan Dabros,
Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski,
Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On Wed, Apr 24, 2024 at 05:26:41PM -0700, Florian Fainelli wrote:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
...
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -17,6 +17,7 @@
> #include <linux/clk-provider.h>
> #include <linux/dmi.h>
> #include <linux/i2c.h>
> +#include <linux/platform_data/i2c-designware.h>
Make it a separate group after linux/* ...
> #include <linux/property.h>
>
...here.
...
> -#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
> +#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
I'm not fan of this, but I think creating another macro to help with
constant device instance naming might be more cumbersome and overkill.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 4/4] net: txgbe: Utilize i2c-designware.h
2024-04-25 0:26 ` [PATCH v2 4/4] net: txgbe: " Florian Fainelli
@ 2024-04-25 9:44 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2024-04-25 9:44 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Jarkko Nikula, Mika Westerberg, Jan Dabros,
Andi Shyti, Lee Jones, Jiawen Wu, Mengyuan Lou, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski,
Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On Wed, Apr 24, 2024 at 05:26:42PM -0700, Florian Fainelli wrote:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
...
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -8,6 +8,7 @@
> #include <linux/clkdev.h>
> #include <linux/i2c.h>
> #include <linux/pci.h>
> +#include <linux/platform_data/i2c-designware.h>
Same comment, make this a separate group, it will be easier to see the quite
specific niche headers, that are not related to the generic libraries / or
subsystem-level ones.
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> #include <linux/pcs/pcs-xpcs.h>
...
> - snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
> - pci_dev_id(pdev));
> + snprintf(clk_name, sizeof(clk_name), "%s.%d",
> + I2C_DESIGNWARE_NAME, pci_dev_id(pdev));
Why do you make it %s? It was constant literal and is, no need to use %s.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/4] Define i2c_designware in a header file
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
` (3 preceding siblings ...)
2024-04-25 0:26 ` [PATCH v2 4/4] net: txgbe: " Florian Fainelli
@ 2024-04-25 13:11 ` Jarkko Nikula
4 siblings, 0 replies; 11+ messages in thread
From: Jarkko Nikula @ 2024-04-25 13:11 UTC (permalink / raw)
To: Florian Fainelli, linux-kernel
Cc: Andy Shevchenko, Mika Westerberg, Jan Dabros, Andi Shyti,
Lee Jones, Jiawen Wu, Mengyuan Lou, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski, Andrew Lunn,
Duanqiang Wen, open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On 4/25/24 3:26 AM, Florian Fainelli wrote:
> This patch series depends upon the following two patches being applied:
>
> https://lore.kernel.org/all/20240422084109.3201-1-duanqiangwen@net-swift.com/
> https://lore.kernel.org/all/20240422084109.3201-2-duanqiangwen@net-swift.com/
>
> There is no reason why each driver should have to repeat the
> "i2c_designware" string all over the place, because when that happens we
> see the reverts like the above being necessary.
>
> Changes in v2:
>
> - avoid changing i2c-designware-pcidrv.c more than necessary
> - move constant to include/linux/platform_data/i2c-designware.h
> - add comments as to how this constant is used and why
>
> Florian Fainelli (4):
> i2c: designware: Create shared header hosting driver name
> mfd: intel-lpss: Utilize i2c-designware.h
> mfd: intel_quark_i2c_gpio: Utilize i2c-designware.h
> net: txgbe: Utilize i2c-designware.h
>
> MAINTAINERS | 1 +
> drivers/i2c/busses/i2c-designware-pcidrv.c | 3 ++-
> drivers/i2c/busses/i2c-designware-platdrv.c | 5 +++--
> drivers/mfd/intel-lpss.c | 3 ++-
> drivers/mfd/intel_quark_i2c_gpio.c | 5 +++--
> drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 7 ++++---
> include/linux/platform_data/i2c-designware.h | 11 +++++++++++
> 7 files changed, 26 insertions(+), 9 deletions(-)
> create mode 100644 include/linux/platform_data/i2c-designware.h
>
I have mixed feeling about this set will it help maintaining and
developing new code or do the opposite. Surely misnaming as referenced
above can happen but I think it's not too common pattern while having
single define header put under include/ feels added burden.
Also I personally like explicit strings put into MODULE_ALIAS() since
they are easier to grep from sources and modules.alias file when
debugging autoloading issues. So change like below makes that debugging
one step more labor.
-MODULE_ALIAS("platform:i2c_designware");
+MODULE_ALIAS("platform:" I2C_DESIGNWARE_NAME);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h
2024-04-25 0:26 ` [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h Florian Fainelli
2024-04-25 9:34 ` Andy Shevchenko
@ 2024-05-02 10:00 ` Lee Jones
1 sibling, 0 replies; 11+ messages in thread
From: Lee Jones @ 2024-05-02 10:00 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-kernel, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Jan Dabros, Andi Shyti, Jiawen Wu, Mengyuan Lou, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maciej Fijalkowski,
Andrew Lunn, Duanqiang Wen,
open list:SYNOPSYS DESIGNWARE I2C DRIVER,
open list:WANGXUN ETHERNET DRIVER
On Wed, 24 Apr 2024, Florian Fainelli wrote:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
>
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
> drivers/mfd/intel-lpss.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 2a9018112dfc..551560beff7b 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -24,6 +24,7 @@
> #include <linux/ioport.h>
> #include <linux/mfd/core.h>
> #include <linux/module.h>
> +#include <linux/platform_data/i2c-designware.h>
> #include <linux/pm.h>
> #include <linux/pm_qos.h>
> #include <linux/pm_runtime.h>
> @@ -116,7 +117,7 @@ static const struct mfd_cell intel_lpss_idma64_cell = {
> };
>
> static const struct mfd_cell intel_lpss_i2c_cell = {
> - .name = "i2c_designware",
> + .name = I2C_DESIGNWARE_NAME,
> .num_resources = ARRAY_SIZE(intel_lpss_dev_resources),
> .resources = intel_lpss_dev_resources,
I explained why I don't like this in v1 this morning.
Please take a look.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-02 10:00 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-25 0:26 [PATCH v2 0/4] Define i2c_designware in a header file Florian Fainelli
2024-04-25 0:26 ` [PATCH v2 1/4] i2c: designware: Create shared header hosting driver name Florian Fainelli
2024-04-25 9:39 ` Andy Shevchenko
2024-04-25 0:26 ` [PATCH v2 2/4] mfd: intel-lpss: Utilize i2c-designware.h Florian Fainelli
2024-04-25 9:34 ` Andy Shevchenko
2024-05-02 10:00 ` Lee Jones
2024-04-25 0:26 ` [PATCH v2 3/4] mfd: intel_quark_i2c_gpio: " Florian Fainelli
2024-04-25 9:42 ` Andy Shevchenko
2024-04-25 0:26 ` [PATCH v2 4/4] net: txgbe: " Florian Fainelli
2024-04-25 9:44 ` Andy Shevchenko
2024-04-25 13:11 ` [PATCH v2 0/4] Define i2c_designware in a header file Jarkko Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).