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 | 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..5ddc9a57ca73 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/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
Tue, Apr 23, 2024 at 04:36:21PM -0700, Florian Fainelli kirjoitti:
> Rather than open code the i2c_designware string, utilize the newly
> defined constant in i2c-designware.h.
..
> -#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
> +#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
So, if you build a module separately for older version of the kernel (assuming
it allows you to modprobe), this won't work anymore.
--
With Best Regards,
Andy Shevchenko
On 4/23/2024 5:01 PM, Andy Shevchenko wrote:
> Tue, Apr 23, 2024 at 04:36:21PM -0700, Florian Fainelli kirjoitti:
>> Rather than open code the i2c_designware string, utilize the newly
>> defined constant in i2c-designware.h.
>
> ...
>
>> -#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
>> +#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
>
> So, if you build a module separately for older version of the kernel (assuming
> it allows you to modprobe), this won't work anymore.
>
Sorry not following, was that comment supposed to be for patch #1 where
I changed the i2c-designware-pci to i2c_designware-pci? modprobe
recognizes both - and _ as interchangeable BTW.
--
Florian
On Wed, Apr 24, 2024 at 4:28 AM Florian Fainelli
<[email protected]> wrote:
> On 4/23/2024 5:01 PM, Andy Shevchenko wrote:
> > Tue, Apr 23, 2024 at 04:36:21PM -0700, Florian Fainelli kirjoitti:
> >> Rather than open code the i2c_designware string, utilize the newly
> >> defined constant in i2c-designware.h.
..
> >> -#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
> >> +#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
> >
> > So, if you build a module separately for older version of the kernel (assuming
> > it allows you to modprobe), this won't work anymore.
>
> Sorry not following, was that comment supposed to be for patch #1 where
> I changed the i2c-designware-pci to i2c_designware-pci? modprobe
> recognizes both - and _ as interchangeable BTW.
I'm talking about something different. Let's assume you have a running
kernel (w.o. signature or version requirement for the modules), then
you have a new patch on top of it and then for an unknown reason you
changed. e.g., designware to DW in that definition. The newly built
module may not be loaded on the running kernel. Also note, here is the
instance name and not an ID in use. The replacement is wrong
semantically.
--
With Best Regards,
Andy Shevchenko
On 4/24/24 05:37, Andy Shevchenko wrote:
> On Wed, Apr 24, 2024 at 4:28 AM Florian Fainelli
> <[email protected]> wrote:
>> On 4/23/2024 5:01 PM, Andy Shevchenko wrote:
>>> Tue, Apr 23, 2024 at 04:36:21PM -0700, Florian Fainelli kirjoitti:
>>>> Rather than open code the i2c_designware string, utilize the newly
>>>> defined constant in i2c-designware.h.
>
> ...
>
>>>> -#define INTEL_QUARK_I2C_CONTROLLER_CLK "i2c_designware.0"
>>>> +#define INTEL_QUARK_I2C_CONTROLLER_CLK I2C_DESIGNWARE_NAME ".0"
>>>
>>> So, if you build a module separately for older version of the kernel (assuming
>>> it allows you to modprobe), this won't work anymore.
>>
>> Sorry not following, was that comment supposed to be for patch #1 where
>> I changed the i2c-designware-pci to i2c_designware-pci? modprobe
>> recognizes both - and _ as interchangeable BTW.
>
> I'm talking about something different. Let's assume you have a running
> kernel (w.o. signature or version requirement for the modules), then
> you have a new patch on top of it and then for an unknown reason you
> changed. e.g., designware to DW in that definition. The newly built
> module may not be loaded on the running kernel. Also note, here is the
> instance name and not an ID in use. The replacement is wrong
> semantically.
>
See my response in the cover letter, the instance base name is not
independent from the i2c-designware-platdrv::driver::name because
otherwise the clock lookup done by devm_clk_get_optional() will fail. So
that change in this patch is entirely intentional and actually ensures
correctness if someone were to change the i2c_designware platform driver
name in the future for whatever reasons.
As far as catering to the specific example you gave, is not this just
fraught with peril regardless of what is being changed in the kernel?
Any constant that is serves as a contract between independent parts
getting out of sync will result in some misbehavior. The only solution
that I can think of which is edging towards over engineering is to
export a string symbol which contains I2C_DESIGNWARE_NAME and then make
other modules dependent upon that symbol to enforce some sort of runtime
resolution, though I think your example could still be made to fail.
--
Florian