This patch series depends upon the following two patches being applied:
https://lore.kernel.org/all/[email protected]/
https://lore.kernel.org/all/[email protected]/
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.
Given the dependency on the two other patches above, it would make sense
to route this via the networking tree, or wait until a v6.9-rc
containing the above two changes gets merged into one of the i2c/mfd
trees.
Changes in v3:
- incorporate Andy's change removing the MODULE_ALIAS
- created a separate inclusion group as requested by Andy
- changed the string format in txgbe_phy.c
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
Andy Shevchenko (1):
i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
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 | 2 --
drivers/i2c/busses/i2c-designware-platdrv.c | 12 +++++++++---
drivers/mfd/intel-lpss.c | 3 ++-
drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 6 ++++--
include/linux/platform_data/i2c-designware.h | 12 ++++++++++++
7 files changed, 32 insertions(+), 10 deletions(-)
create mode 100644 include/linux/platform_data/i2c-designware.h
--
2.34.1
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/mfd/intel_quark_i2c_gpio.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 9b9c76bd067b..86620de5c9ba 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -19,6 +19,8 @@
#include <linux/i2c.h>
#include <linux/property.h>
+#include <linux/platform_data/i2c-designware.h>
+
/* PCI BAR for register base address */
#define MFD_I2C_BAR 0
#define MFD_GPIO_BAR 1
@@ -30,7 +32,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 +138,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
From: Andy Shevchenko <[email protected]>
As Krzysztof Kozlowski pointed out the better is to use
MODULE_DEVICE_TABLE() as it will be consistent with the content
of the real ID table of the platform devices.
While at it, drop unneeded and unused module alias in PCI glue
driver as PCI already has its own ID table and automatic loading
should just work.
Reviewed-by: Andi Shyti <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
Acked-by: Jarkko Nikula <[email protected]>
Tested-by: Jarkko Nikula <[email protected]>
Tested-by: Serge Semin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/i2c/busses/i2c-designware-pcidrv.c | 2 --
drivers/i2c/busses/i2c-designware-platdrv.c | 8 ++++++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 9be9a2658e1f..a1b379a1e904 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -424,8 +424,6 @@ static struct pci_driver dw_i2c_driver = {
};
module_pci_driver(dw_i2c_driver);
-/* Work with hotplug and coldplug */
-MODULE_ALIAS("i2c_designware-pci");
MODULE_AUTHOR("Baruch Siach <[email protected]>");
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..0be7b0dc849b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -479,8 +479,11 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend, dw_i2c_plat_runtime_resume, NULL)
};
-/* Work with hotplug and coldplug */
-MODULE_ALIAS("platform:i2c_designware");
+static const struct platform_device_id dw_i2c_platform_ids[] = {
+ { "i2c_designware" },
+ {}
+};
+MODULE_DEVICE_TABLE(platform, dw_i2c_platform_ids);
static struct platform_driver dw_i2c_driver = {
.probe = dw_i2c_plat_probe,
@@ -491,6 +494,7 @@ static struct platform_driver dw_i2c_driver = {
.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
.pm = pm_ptr(&dw_i2c_dev_pm_ops),
},
+ .id_table = dw_i2c_platform_ids,
};
static int __init dw_i2c_init_driver(void)
--
2.34.1
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Acked-by: Andy Shevchenko <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
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..1327a6a07d1f 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -33,6 +33,7 @@
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/dma/idma64.h>
+#include <linux/platform_data/i2c-designware.h>
#include "intel-lpss.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
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.
Link: https://lore.kernel.org/all/[email protected]/ [1]
Link: https://lore.kernel.org/all/[email protected]/ [2]
Signed-off-by: Florian Fainelli <[email protected]>
---
MAINTAINERS | 1 +
drivers/i2c/busses/i2c-designware-platdrv.c | 4 +++-
include/linux/platform_data/i2c-designware.h | 12 ++++++++++++
3 files changed, 16 insertions(+), 1 deletion(-)
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 <[email protected]>
L: [email protected]
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 <[email protected]>
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0be7b0dc849b..107945d0337e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -33,6 +33,8 @@
#include <linux/suspend.h>
#include <linux/units.h>
+#include <linux/platform_data/i2c-designware.h>
+
#include "i2c-designware-core.h"
static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
@@ -489,7 +491,7 @@ 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..1196103df4f5
--- /dev/null
+++ b/include/linux/platform_data/i2c-designware.h
@@ -0,0 +1,12 @@
+/* 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
Rather than open code the i2c_designware string, utilize the newly
defined constant in i2c-designware.h.
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 93295916b1d2..01a442c38248 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -13,6 +13,8 @@
#include <linux/pcs/pcs-xpcs.h>
#include <linux/phylink.h>
+#include <linux/platform_data/i2c-designware.h>
+
#include "../libwx/wx_type.h"
#include "../libwx/wx_lib.h"
#include "../libwx/wx_hw.h"
@@ -571,7 +573,7 @@ static int txgbe_clock_register(struct txgbe *txgbe)
char clk_name[32];
struct clk *clk;
- snprintf(clk_name, sizeof(clk_name), "i2c_designware.%d",
+ snprintf(clk_name, sizeof(clk_name), I2C_DESIGNWARE_NAME ".%d",
pci_dev_id(pdev));
clk = clk_register_fixed_rate(NULL, clk_name, NULL, 0, 156250000);
@@ -634,7 +636,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
On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
Acked-by: Andy Shevchenko <[email protected]>
P.S>
Note, my tags for MFD patches does not imply that I agree on the general idea
of this series, it's just in case if it will be approved by the respective
maintainers (I?C / MFD / etc).
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
> > Rather than open code the i2c_designware string, utilize the newly
> > defined constant in i2c-designware.h.
>
> Acked-by: Andy Shevchenko <[email protected]>
>
> P.S>
> Note, my tags for MFD patches does not imply that I agree on the general idea
> of this series, it's just in case if it will be approved by the respective
> maintainers (I?C / MFD / etc).
I waited a bit to see if more comments were coming.
Do you have a better approach in mind?
Andi
On 4/30/24 12:36 PM, Andi Shyti wrote:
> Hi Andy,
>
> On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
>> On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
>>> Rather than open code the i2c_designware string, utilize the newly
>>> defined constant in i2c-designware.h.
>>
>> Acked-by: Andy Shevchenko <[email protected]>
>>
>> P.S>
>> Note, my tags for MFD patches does not imply that I agree on the general idea
>> of this series, it's just in case if it will be approved by the respective
>> maintainers (I²C / MFD / etc).
>
> I waited a bit to see if more comments were coming.
>
Well I had doubts about the idea will it help maintaining code or do the
opposite but didn't receive a reply from the patch author:
https://lore.kernel.org/linux-i2c/[email protected]/
Also Lee Jones have similar concerns:
https://lore.kernel.org/linux-i2c/[email protected]/
On Thu, 02 May 2024, Jarkko Nikula wrote:
> On 4/30/24 12:36 PM, Andi Shyti wrote:
> > Hi Andy,
> >
> > On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
> > > On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
> > > > Rather than open code the i2c_designware string, utilize the newly
> > > > defined constant in i2c-designware.h.
> > >
> > > Acked-by: Andy Shevchenko <[email protected]>
> > >
> > > P.S>
> > > Note, my tags for MFD patches does not imply that I agree on the general idea
> > > of this series, it's just in case if it will be approved by the respective
> > > maintainers (I²C / MFD / etc).
> >
> > I waited a bit to see if more comments were coming.
> >
> Well I had doubts about the idea will it help maintaining code or do the
> opposite but didn't receive a reply from the patch author:
>
> https://lore.kernel.org/linux-i2c/[email protected]/
>
> Also Lee Jones have similar concerns:
>
> https://lore.kernel.org/linux-i2c/[email protected]/
Right. It's a NACK from me, sorry.
--
Lee Jones [李琼斯]
On 5/2/24 02:19, Jarkko Nikula wrote:
> On 4/30/24 12:36 PM, Andi Shyti wrote:
>> Hi Andy,
>>
>> On Fri, Apr 26, 2024 at 05:30:24PM +0300, Andy Shevchenko wrote:
>>> On Thu, Apr 25, 2024 at 02:44:37PM -0700, Florian Fainelli wrote:
>>>> Rather than open code the i2c_designware string, utilize the newly
>>>> defined constant in i2c-designware.h.
>>>
>>> Acked-by: Andy Shevchenko <[email protected]>
>>>
>>> P.S>
>>> Note, my tags for MFD patches does not imply that I agree on the
>>> general idea
>>> of this series, it's just in case if it will be approved by the
>>> respective
>>> maintainers (I²C / MFD / etc).
>>
>> I waited a bit to see if more comments were coming.
>>
> Well I had doubts about the idea will it help maintaining code or do the
> opposite but didn't receive a reply from the patch author:
>
> https://lore.kernel.org/linux-i2c/[email protected]/
I read your message and the fact that you provided a diff as a
suggestion as to what would become acceptable if incorporating your
suggested change, and made that a v3. If this was not acceptable to you
from the get go, it would have been clearer with an explicit Nack like
what others have done now.
>
> Also Lee Jones have similar concerns:
>
> https://lore.kernel.org/linux-i2c/[email protected]/
>
Yes, so clearly I failed to convince all of you that if someone was able
to trip over that problem, something should be done about it. No
problem, these are not driver I maintain, so if someone hits the same
issue, lore has the patches now.
--
Florian
Hi
On Thu, 25 Apr 2024 14:44:33 -0700, Florian Fainelli wrote:
> This patch series depends upon the following two patches being applied:
>
> https://lore.kernel.org/all/[email protected]/
> https://lore.kernel.org/all/[email protected]/
>
> 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.
>
> [...]
Applied to i2c/i2c-host on
git://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git
Thank you,
Andi
Patches applied
===============
[1/5] i2c: designware: Replace MODULE_ALIAS() with MODULE_DEVICE_TABLE()
commit: 91647e64f0f5677ace84165dc25dc99579147b8f
[2/5] i2c: designware: Create shared header hosting driver name
commit: 856cd5f13de7cebca44db5ff4bc2ca73490dd8d7