Hi Andy,
On Mon, Jul 31, 2023 at 11:05:05PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 28, 2023 at 03:33:59PM +0300, Jarkko Nikula wrote:
> > On 7/25/23 17:30, Andy Shevchenko wrote:
>
> ...
>
> > > +/* Work with hotplug and coldplug */
> > > +MODULE_ALIAS("platform:i2c_designware");
> >
> > Perhaps this comment can be retired, i.e. dropped.
>
> Then it needs to be done in a separate patch, because in the other file the
> comment will be left untouched.
You are being a bit too religios here... if you want to stick to
this, then you need to send a patch for sorting by ID, a patch
for grouping together MODULE_*, a patch to remove this comment
and a patch to always provide the id table.
I think, "while at it", you can safely remove the redundant
comment :)
It doesn't make too much difference to me anyway:
Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Andi
On Fri, Aug 04, 2023 at 11:00:55PM +0200, Andi Shyti wrote:
> On Mon, Jul 31, 2023 at 11:05:05PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 28, 2023 at 03:33:59PM +0300, Jarkko Nikula wrote:
> > > On 7/25/23 17:30, Andy Shevchenko wrote:
...
> > > > +/* Work with hotplug and coldplug */
> > > > +MODULE_ALIAS("platform:i2c_designware");
> > >
> > > Perhaps this comment can be retired, i.e. dropped.
> >
> > Then it needs to be done in a separate patch, because in the other file the
> > comment will be left untouched.
>
> You are being a bit too religios here...
No, it's being consistent. Either we remove them both or don't touch.
> if you want to stick to
> this, then you need to send a patch for sorting by ID, a patch
> for grouping together MODULE_*, a patch to remove this comment
> and a patch to always provide the id table.
>
> I think, "while at it", you can safely remove the redundant
> comment :)
>
> It doesn't make too much difference to me anyway:
>
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
Thank you!
@@ -40,28 +40,6 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
return clk_get_rate(dev->clk) / KILO;
}
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id dw_i2c_acpi_match[] = {
- { "INT33C2", 0 },
- { "INT33C3", 0 },
- { "INT3432", 0 },
- { "INT3433", 0 },
- { "80860F41", ACCESS_NO_IRQ_SUSPEND },
- { "808622C1", ACCESS_NO_IRQ_SUSPEND },
- { "AMD0010", ACCESS_INTR_MASK },
- { "AMDI0010", ACCESS_INTR_MASK },
- { "AMDI0019", ACCESS_INTR_MASK | ARBITRATION_SEMAPHORE },
- { "AMDI0510", 0 },
- { "APMC0D0F", 0 },
- { "HISI02A1", 0 },
- { "HISI02A2", 0 },
- { "HISI02A3", 0 },
- { "HYGO0010", ACCESS_INTR_MASK },
- { }
-};
-MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-#endif
-
#ifdef CONFIG_OF
#define BT1_I2C_CTL 0x100
#define BT1_I2C_CTL_ADDR_MASK GENMASK(7, 0)
@@ -152,14 +130,6 @@ static void i2c_dw_of_configure(struct dw_i2c_dev *dev)
if (dev_of_node(dev->dev))
i2c_dw_of_do_configure(dev, dev->dev);
}
-
-static const struct of_device_id dw_i2c_of_match[] = {
- { .compatible = "snps,designware-i2c", },
- { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
- { .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
- {},
-};
-MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
#else
static int bt1_i2c_request_regs(struct dw_i2c_dev *dev)
{
@@ -485,16 +455,41 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
#define DW_I2C_DEV_PMOPS NULL
#endif
-/* Work with hotplug and coldplug */
-MODULE_ALIAS("platform:i2c_designware");
+static const struct of_device_id dw_i2c_of_match[] = {
+ { .compatible = "snps,designware-i2c", },
+ { .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+ { .compatible = "baikal,bt1-sys-i2c", .data = (void *)MODEL_BAIKAL_BT1 },
+ {}
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+
+static const struct acpi_device_id dw_i2c_acpi_match[] = {
+ { "80860F41", ACCESS_NO_IRQ_SUSPEND },
+ { "808622C1", ACCESS_NO_IRQ_SUSPEND },
+ { "AMD0010", ACCESS_INTR_MASK },
+ { "AMDI0010", ACCESS_INTR_MASK },
+ { "AMDI0019", ACCESS_INTR_MASK | ARBITRATION_SEMAPHORE },
+ { "AMDI0510", 0 },
+ { "APMC0D0F", 0 },
+ { "HISI02A1", 0 },
+ { "HISI02A2", 0 },
+ { "HISI02A3", 0 },
+ { "HYGO0010", ACCESS_INTR_MASK },
+ { "INT33C2", 0 },
+ { "INT33C3", 0 },
+ { "INT3432", 0 },
+ { "INT3433", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
static struct platform_driver dw_i2c_driver = {
.probe = dw_i2c_plat_probe,
.remove_new = dw_i2c_plat_remove,
.driver = {
.name = "i2c_designware",
- .of_match_table = of_match_ptr(dw_i2c_of_match),
- .acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
+ .of_match_table = dw_i2c_of_match,
+ .acpi_match_table = dw_i2c_acpi_match,
.pm = DW_I2C_DEV_PMOPS,
},
};
@@ -511,6 +506,8 @@ static void __exit dw_i2c_exit_driver(void)
}
module_exit(dw_i2c_exit_driver);
+/* Work with hotplug and coldplug */
+MODULE_ALIAS("platform:i2c_designware");
MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>");
MODULE_DESCRIPTION("Synopsys DesignWare I2C bus adapter");
MODULE_LICENSE("GPL");