We use MODEL_MSCC_OCELOT effectively is a flag for comparing against
"compatible" property. Use device_is_compatible() directly to make it
clear.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/i2c/busses/i2c-designware-common.c | 6 +-----
drivers/i2c/busses/i2c-designware-core.h | 1 -
drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
3 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 5b1e8f74c4ac..c766d9821975 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -238,14 +238,10 @@ static void i2c_dw_of_configure(struct device *device)
struct platform_device *pdev = to_platform_device(device);
struct dw_i2c_dev *dev = dev_get_drvdata(device);
- switch (dev->flags & MODEL_MASK) {
- case MODEL_MSCC_OCELOT:
+ if (device_is_compatible(dev->dev, "mscc,ocelot-i2c")) {
dev->ext = devm_platform_ioremap_resource(pdev, 1);
if (!IS_ERR(dev->ext))
dev->set_sda_hold_time = mscc_twi_set_sda_hold_time;
- break;
- default:
- break;
}
}
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index cf0364079b55..10055f0e0ec3 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -313,7 +313,6 @@ struct dw_i2c_dev {
#define ARBITRATION_SEMAPHORE BIT(2)
#define ACCESS_POLLING BIT(3)
-#define MODEL_MSCC_OCELOT BIT(8)
#define MODEL_AMD_NAVI_GPU BIT(10)
#define MODEL_WANGXUN_SP BIT(11)
#define MODEL_MASK GENMASK(11, 8)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 2e532f16691b..4e6fe3b55322 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -267,7 +267,7 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
}
static const struct of_device_id dw_i2c_of_match[] = {
- { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+ { .compatible = "mscc,ocelot-i2c" },
{ .compatible = "snps,designware-i2c" },
{}
};
--
2.50.1Hi, On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > "compatible" property. Use device_is_compatible() directly to make it > clear. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> I'm coming into the discussion and I personally like Andy's approach. As a third party, I believe this patch has reached the consensus and I'm taking it in i2c/i2c-host. Thanks Andy, Andi
On Thu, Jan 22, 2026 at 12:10:48PM +0100, Andi Shyti wrote: > On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > > "compatible" property. Use device_is_compatible() directly to make it > > clear. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > I'm coming into the discussion and I personally like Andy's > approach. As a third party, I believe this patch has reached the > consensus and I'm taking it in i2c/i2c-host. Thank you! -- With Best Regards, Andy Shevchenko
On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote:
> We use MODEL_MSCC_OCELOT effectively is a flag for comparing against
as a flag?
> "compatible" property. Use device_is_compatible() directly to make it
> clear.
Okay but if something else ever needs this same quirk then we would need to
add new entry here and also to the IDs list.
With the flag you can have the IDs in a single place not all over the
driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/i2c/busses/i2c-designware-common.c | 6 +-----
> drivers/i2c/busses/i2c-designware-core.h | 1 -
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> 3 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
> index 5b1e8f74c4ac..c766d9821975 100644
> --- a/drivers/i2c/busses/i2c-designware-common.c
> +++ b/drivers/i2c/busses/i2c-designware-common.c
> @@ -238,14 +238,10 @@ static void i2c_dw_of_configure(struct device *device)
> struct platform_device *pdev = to_platform_device(device);
> struct dw_i2c_dev *dev = dev_get_drvdata(device);
>
> - switch (dev->flags & MODEL_MASK) {
> - case MODEL_MSCC_OCELOT:
> + if (device_is_compatible(dev->dev, "mscc,ocelot-i2c")) {
> dev->ext = devm_platform_ioremap_resource(pdev, 1);
> if (!IS_ERR(dev->ext))
> dev->set_sda_hold_time = mscc_twi_set_sda_hold_time;
> - break;
> - default:
> - break;
> }
> }
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index cf0364079b55..10055f0e0ec3 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -313,7 +313,6 @@ struct dw_i2c_dev {
> #define ARBITRATION_SEMAPHORE BIT(2)
> #define ACCESS_POLLING BIT(3)
>
> -#define MODEL_MSCC_OCELOT BIT(8)
> #define MODEL_AMD_NAVI_GPU BIT(10)
> #define MODEL_WANGXUN_SP BIT(11)
> #define MODEL_MASK GENMASK(11, 8)
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 2e532f16691b..4e6fe3b55322 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -267,7 +267,7 @@ static void dw_i2c_plat_remove(struct platform_device *pdev)
> }
>
> static const struct of_device_id dw_i2c_of_match[] = {
> - { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
> + { .compatible = "mscc,ocelot-i2c" },
> { .compatible = "snps,designware-i2c" },
> {}
> };
> --
> 2.50.1
On Wed, Jan 14, 2026 at 12:53:04PM +0100, Mika Westerberg wrote: > On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > > as a flag? Yes, sorry for the typo. > > "compatible" property. Use device_is_compatible() directly to make it > > clear. > > Okay but if something else ever needs this same quirk then we would need to > add new entry here and also to the IDs list. Yes, that's how DT works and there are, of course, examples all over the kernel, first that comes to my mind: drivers/mmc/host/sdhci-pltfm.c. > With the flag you can have the IDs in a single place not all over the > driver. Makes the reality harder to read. If I know that the same quirk is used by different platform (in terms of compatible string) I will see it immediately from the code. Flag is meaningless. -- With Best Regards, Andy Shevchenko
On Wed, Jan 14, 2026 at 06:09:09PM +0200, Andy Shevchenko wrote: > On Wed, Jan 14, 2026 at 12:53:04PM +0100, Mika Westerberg wrote: > > On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > > > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > > > > as a flag? > > Yes, sorry for the typo. > > > > "compatible" property. Use device_is_compatible() directly to make it > > > clear. > > > > Okay but if something else ever needs this same quirk then we would need to > > add new entry here and also to the IDs list. > > Yes, that's how DT works and there are, of course, examples all over > the kernel, first that comes to my mind: drivers/mmc/host/sdhci-pltfm.c. > > > With the flag you can have the IDs in a single place not all over the > > driver. > > Makes the reality harder to read. If I know that the same quirk is used by > different platform (in terms of compatible string) I will see it immediately > from the code. Flag is meaningless. Well with the flag you get the help from the compiler if you typo it but with the string comparison you are on your own. Therefore I prefer the flag and as I said it also avoid duplicating the compatible string.
On Thu, Jan 15, 2026 at 06:39:59AM +0100, Mika Westerberg wrote: > On Wed, Jan 14, 2026 at 06:09:09PM +0200, Andy Shevchenko wrote: > > On Wed, Jan 14, 2026 at 12:53:04PM +0100, Mika Westerberg wrote: > > > On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > > > > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > > > > > > as a flag? > > > > Yes, sorry for the typo. > > > > > > "compatible" property. Use device_is_compatible() directly to make it > > > > clear. > > > > > > Okay but if something else ever needs this same quirk then we would need to > > > add new entry here and also to the IDs list. > > > > Yes, that's how DT works and there are, of course, examples all over > > the kernel, first that comes to my mind: drivers/mmc/host/sdhci-pltfm.c. > > > > > With the flag you can have the IDs in a single place not all over the > > > driver. > > > > Makes the reality harder to read. If I know that the same quirk is used by > > different platform (in terms of compatible string) I will see it immediately > > from the code. Flag is meaningless. > > Well with the flag you get the help from the compiler if you typo it but > with the string comparison you are on your own. It only affects the development cycle, isn't it obvious that any quirk should be tested by the author, otherwise it won't work. > Therefore I prefer the flag and as I said it also avoid duplicating the > compatible string. Duplicating compatible string is not a problem, linker nowadays combines the same strings into one. In case you are worrying about the space, it's not a problem. -- With Best Regards, Andy Shevchenko
On Thu, Jan 15, 2026 at 09:41:26AM +0200, Andy Shevchenko wrote: > On Thu, Jan 15, 2026 at 06:39:59AM +0100, Mika Westerberg wrote: > > On Wed, Jan 14, 2026 at 06:09:09PM +0200, Andy Shevchenko wrote: > > > On Wed, Jan 14, 2026 at 12:53:04PM +0100, Mika Westerberg wrote: > > > > On Wed, Jan 14, 2026 at 09:17:51AM +0100, Andy Shevchenko wrote: > > > > > We use MODEL_MSCC_OCELOT effectively is a flag for comparing against > > > > > > > > as a flag? > > > > > > Yes, sorry for the typo. > > > > > > > > "compatible" property. Use device_is_compatible() directly to make it > > > > > clear. > > > > > > > > Okay but if something else ever needs this same quirk then we would need to > > > > add new entry here and also to the IDs list. > > > > > > Yes, that's how DT works and there are, of course, examples all over > > > the kernel, first that comes to my mind: drivers/mmc/host/sdhci-pltfm.c. > > > > > > > With the flag you can have the IDs in a single place not all over the > > > > driver. > > > > > > Makes the reality harder to read. If I know that the same quirk is used by > > > different platform (in terms of compatible string) I will see it immediately > > > from the code. Flag is meaningless. > > > > Well with the flag you get the help from the compiler if you typo it but > > with the string comparison you are on your own. > > It only affects the development cycle, isn't it obvious that any quirk should > be tested by the author, otherwise it won't work. Of course but then are these "cleanups" that probably will not be tested that well if at all. If it is possible to take advantage of the compiler then I prefer that. > > Therefore I prefer the flag and as I said it also avoid duplicating the > > compatible string. > > Duplicating compatible string is not a problem, linker nowadays combines > the same strings into one. In case you are worrying about the space, it's > not a problem. I don't care about the space but just duplication.
© 2016 - 2026 Red Hat, Inc.