rtc: ds1307: Remove non-valid ACPI IDs
Subsystems (2)
PATCH v2 1/3 rtc: ds1307: Remove non-valid ACPI IDs
The commit 9c19b8930d2c ("rtc: ds1307: Add ACPI support") added invalid ACPI IDs (all of them are abusing ACPI specification). Moreover there is not even a single evidence that vendor registered any of such devices.
Remove broken ACPI IDs from the driver. For prototyping one may use PRP0001 with device properties adhering to a DT binding. The following patches will add support of that to the driver.
---
v2: reworded commit message to clarify source of properties (Rafael)
drivers/rtc/rtc-ds1307.c | 36 +----------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-)
drivers/rtc/rtc-ds1307.c +1 -35
@@ -8,7 +8,6 @@* Copyright (C) 2012 Bertrand Achard (nvram access fixes) */ -#include <linux/acpi.h> #include <linux/bcd.h> #include <linux/i2c.h> #include <linux/init.h>
@@ -1169,31 +1168,6 @@ static const struct of_device_id ds1307_of_match[] = {MODULE_DEVICE_TABLE (of, ds1307_of_match);#endif -#ifdef CONFIG_ACPI -static const struct acpi_device_id ds1307_acpi_ids[] = { - { .id ="DS1307" , .driver_data = ds_1307 }, - { .id ="DS1308" , .driver_data = ds_1308 }, - { .id ="DS1337" , .driver_data = ds_1337 }, - { .id ="DS1338" , .driver_data = ds_1338 }, - { .id ="DS1339" , .driver_data = ds_1339 }, - { .id ="DS1388" , .driver_data = ds_1388 }, - { .id ="DS1340" , .driver_data = ds_1340 }, - { .id ="DS1341" , .driver_data = ds_1341 }, - { .id ="DS3231" , .driver_data = ds_3231 }, - { .id ="M41T0" , .driver_data = m41t0 }, - { .id ="M41T00" , .driver_data = m41t00 }, - { .id ="M41T11" , .driver_data = m41t11 }, - { .id ="MCP7940X" , .driver_data = mcp794xx }, - { .id ="MCP7941X" , .driver_data = mcp794xx }, - { .id ="PT7C4338" , .driver_data = ds_1307 }, - { .id ="RX8025" , .driver_data = rx_8025 }, - { .id ="ISL12057" , .driver_data = ds_1337 }, - { .id ="RX8130" , .driver_data = rx_8130 }, - { } -}; -MODULE_DEVICE_TABLE (acpi, ds1307_acpi_ids); -#endif -/* * The ds1337 and ds1339 both have two alarms, but we only use the first * one (with a "seconds" field). For ds1337 we expect nINTA is our alarm
@@ -1794,14 +1768,7 @@ static int ds1307_probe(struct i2c_client *client, chip = &chips[id->driver_data]; ds1307->type = id->driver_data; }else { -const struct acpi_device_id *acpi_id; - - acpi_id =acpi_match_device (ACPI_PTR (ds1307_acpi_ids), - ds1307->dev); -if (!acpi_id) -return -ENODEV ; - chip = &chips[acpi_id->driver_data]; - ds1307->type = acpi_id->driver_data; +return -ENODEV ; } want_irq = client->irq >0 && chip->alarm;
@@ -2065,7 +2032,6 @@ static struct i2c_driver ds1307_driver = { .driver = { .name = "rtc-ds1307", .of_match_table = of_match_ptr(ds1307_of_match), - .acpi_match_table = ACPI_PTR(ds1307_acpi_ids), }, .probe = ds1307_probe, .id_table = ds1307_id,
Signature · 1 line
2.28.0
PATCH v2 2/3 rtc: ds1307: Make use of device properties
Device property API allows to gather device resources from different sources, such as ACPI. Convert the drivers to unleash the power of device property API.
---
v2: moved ds3231_clks_names to be global and static
drivers/rtc/rtc-ds1307.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
drivers/rtc/rtc-ds1307.c +21 -17
@@ -12,7 +12,8 @@#include <linux/i2c.h> #include <linux/init.h> #include <linux/module.h> -#include <linux/of_device.h> +#include <linux/of.h> +#include <linux/property.h> #include <linux/rtc/ds1307.h> #include <linux/rtc.h> #include <linux/slab.h>
@@ -30,6 +31,7 @@* That's a natural job for a factory or repair bench. */ enum ds_type { + unknown_ds_type,/* always first and 0 */ ds_1307, ds_1308, ds_1337,
@@ -1600,13 +1602,16 @@ static const struct clk_ops ds3231_clk_32khz_ops = { .recalc_rate = ds3231_clk_32khz_recalc_rate, }; +static const char *ds3231_clks_names[] = { + [DS3231_CLK_SQW ] ="ds3231_clk_sqw" , + [DS3231_CLK_32KHZ ] ="ds3231_clk_32khz" , +}; +static struct clk_init_data ds3231_clks_init[] = { [DS3231_CLK_SQW ] = { - .name ="ds3231_clk_sqw" , .ops = &ds3231_clk_sqw_ops, }, [DS3231_CLK_32KHZ ] = { - .name ="ds3231_clk_32khz" , .ops = &ds3231_clk_32khz_ops, }, };
@@ -1627,6 +1632,11 @@ static int ds3231_clks_register(struct ds1307 *ds1307)if (!onecell->clks)return -ENOMEM ; +/* optional override of the clockname */ +device_property_read_string_array (ds1307->dev,"clock-output-names" , + ds3231_clks_names, +ARRAY_SIZE (ds3231_clks_names)); +for (i =0 ; i <ARRAY_SIZE (ds3231_clks_init); i++) {struct clk_init_data init = ds3231_clks_init[i];
@@ -1637,9 +1647,7 @@ static int ds3231_clks_register(struct ds1307 *ds1307)if (i ==DS3231_CLK_SQW &&test_bit (HAS_ALARM , &ds1307->flags))continue ; -/* optional override of the clockname */ -of_property_read_string_index (node,"clock-output-names" , i, - &init.name); + init.name = ds3231_clks_names[i]; ds1307->clks[i].init = &init; onecell->clks[i] = devm_clk_register(ds1307->dev,
@@ -1648,10 +1656,8 @@ static int ds3231_clks_register(struct ds1307 *ds1307)return PTR_ERR (onecell->clks[i]); } -if (!node) -return 0 ; - -of_clk_add_provider (node, of_clk_src_onecell_get, onecell); +if (node) +of_clk_add_provider (node, of_clk_src_onecell_get, onecell);return 0 ; }
@@ -1735,6 +1741,7 @@ static int ds1307_probe(struct i2c_client *client, const struct i2c_device_id *id) {struct ds1307 *ds1307; +const void *match;int err = -ENODEV ;int tmp;const struct chip_desc *chip;
@@ -1760,9 +1767,9 @@ static int ds1307_probe(struct i2c_client *client,i2c_set_clientdata (client, ds1307); -if (client->dev.of_node) { - ds1307->type = (enum ds_type ) -of_device_get_match_data (&client->dev); + match =device_get_match_data (&client->dev); +if (match) { + ds1307->type = (enum ds_type )match; chip = &chips[ds1307->type]; }else if (id) { chip = &chips[id->driver_data];
@@ -1786,7 +1793,6 @@ static int ds1307_probe(struct i2c_client *client, trickle_charger_setup); } -#ifdef CONFIG_OF /* * For devices with no IRQ directly connected to the SoC, the RTC chip * can be forced as a wakeup source by stating that explicitly in
@@ -1795,10 +1801,8 @@ static int ds1307_probe(struct i2c_client *client,* This will guarantee the 'wakealarm' sysfs entry is available on the device, * if supported by the RTC. */ -if (chip->alarm &&of_property_read_bool (client->dev.of_node, -"wakeup-source" )) +if (chip->alarm &&device_property_read_bool (&client->dev,"wakeup-source" )) ds1307_can_wakeup_device = true; -#endif switch (ds1307->type) {case ds_1337:
Signature · 1 line
2.28.0
PATCH v2 3/3 rtc: ds1307: Drop of_match_ptr and CONFIG_OF protections
In reply to Andy ShevchenkoThese prevent use of this driver with ACPI via PRP0001. Drop them to remove this restriction.
Also added mod_devicetable.h include given use of struct of_device_id.
---
v2: no changes
drivers/rtc/rtc-ds1307.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/rtc/rtc-ds1307.c +2 -4
@@ -11,8 +11,8 @@#include <linux/bcd.h> #include <linux/i2c.h> #include <linux/init.h> +#include <linux/mod_devicetable.h> #include <linux/module.h> -#include <linux/of.h> #include <linux/property.h> #include <linux/rtc/ds1307.h> #include <linux/rtc.h>
@@ -1091,7 +1091,6 @@ static const struct i2c_device_id ds1307_id[] = { };MODULE_DEVICE_TABLE (i2c, ds1307_id); -#ifdef CONFIG_OFstatic const struct of_device_id ds1307_of_match[] = { { .compatible ="dallas,ds1307" ,
@@ -1168,7 +1167,6 @@ static const struct of_device_id ds1307_of_match[] = { { } };MODULE_DEVICE_TABLE (of, ds1307_of_match); -#endif /* * The ds1337 and ds1339 both have two alarms, but we only use the first
@@ -2035,7 +2033,7 @@ static int ds1307_probe(struct i2c_client *client,static struct i2c_driver ds1307_driver = { .driver = { .name ="rtc-ds1307" , - .of_match_table =of_match_ptr (ds1307_of_match), + .of_match_table = ds1307_of_match, }, .probe = ds1307_probe, .id_table = ds1307_id,
Signature · 1 line
2.28.0
Quoted 5 lines · Andy Shevchenko
The commit
9c19b8930d2c("rtc: ds1307: Add ACPI support") added invalid ACPI IDs (all of them are abusing ACPI specification). Moreover there is not even a single evidence that vendor registered any of such devices.
Quoted 7 lines
Remove broken ACPI IDs from the driver. For prototyping one may use PRP0001 with device properties adhering to a DT binding. The following patches will add support of that to the driver.
Cc Tin Huynh <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>
FWIW, the series looks good to me.
Please feel free to add
to the patches.
Quoted 76 lines
---
v2: reworded commit message to clarify source of properties (Rafael)drivers/rtc/rtc-ds1307.c | 36 +----------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-)drivers/rtc/rtc-ds1307.c +1
-36@@ -8,7 +8,6 @@* Copyright (C) 2012 Bertrand Achard (nvram access fixes) */ -#include <linux/acpi.h> #include <linux/bcd.h> #include <linux/i2c.h> #include <linux/init.h> @@ -1169,31 +1168,6 @@ static const struct of_device_id ds1307_of_match[] = {MODULE_DEVICE_TABLE (of, ds1307_of_match);#endif -#ifdef CONFIG_ACPI -static const struct acpi_device_id ds1307_acpi_ids[] = { - { .id ="DS1307" , .driver_data = ds_1307 }, - { .id ="DS1308" , .driver_data = ds_1308 }, - { .id ="DS1337" , .driver_data = ds_1337 }, - { .id ="DS1338" , .driver_data = ds_1338 }, - { .id ="DS1339" , .driver_data = ds_1339 }, - { .id ="DS1388" , .driver_data = ds_1388 }, - { .id ="DS1340" , .driver_data = ds_1340 }, - { .id ="DS1341" , .driver_data = ds_1341 }, - { .id ="DS3231" , .driver_data = ds_3231 }, - { .id ="M41T0" , .driver_data = m41t0 }, - { .id ="M41T00" , .driver_data = m41t00 }, - { .id ="M41T11" , .driver_data = m41t11 }, - { .id ="MCP7940X" , .driver_data = mcp794xx }, - { .id ="MCP7941X" , .driver_data = mcp794xx }, - { .id ="PT7C4338" , .driver_data = ds_1307 }, - { .id ="RX8025" , .driver_data = rx_8025 }, - { .id ="ISL12057" , .driver_data = ds_1337 }, - { .id ="RX8130" , .driver_data = rx_8130 }, - { } -}; -MODULE_DEVICE_TABLE (acpi, ds1307_acpi_ids); -#endif -/* * The ds1337 and ds1339 both have two alarms, but we only use the first * one (with a "seconds" field). For ds1337 we expect nINTA is our alarm @@ -1794,14 +1768,7 @@ static int ds1307_probe(struct i2c_client *client, chip = &chips[id->driver_data]; ds1307->type = id->driver_data; }else { -const struct acpi_device_id *acpi_id; - - acpi_id =acpi_match_device (ACPI_PTR (ds1307_acpi_ids), - ds1307->dev); -if (!acpi_id) -return -ENODEV ; - chip = &chips[acpi_id->driver_data]; - ds1307->type = acpi_id->driver_data; +return -ENODEV ; } want_irq = client->irq >0 && chip->alarm;@@ -2065,7 +2032,6 @@ static struct i2c_driver ds1307_driver = { .driver = { .name = "rtc-ds1307", .of_match_table = of_match_ptr(ds1307_of_match), - .acpi_match_table = ACPI_PTR(ds1307_acpi_ids), }, .probe = ds1307_probe, .id_table = ds1307_id, --2.28.0
Quoted 7 lines · Andy Shevchenko
The commit
9c19b8930d2c("rtc: ds1307: Add ACPI support") added invalid ACPI IDs (all of them are abusing ACPI specification). Moreover there is not even a single evidence that vendor registered any of such devices.Remove broken ACPI IDs from the driver. For prototyping one may use PRP0001 with device properties adhering to a DT binding. The following patches will add support of that to the driver.
Applied, thanks!
[1/3] rtc: ds1307: Remove non-valid ACPI IDs
commit: a31111189bb1160f84cf4cf9f910aa2ba7553d18
[2/3] rtc: ds1307: Make use of device properties
commit: 227ec129ad7b035ee2ae2e57e9567a8126ad93f3
[3/3] rtc: ds1307: Drop of_match_ptr and CONFIG_OF protections
commit: 698fffc2705cc48804cc31021cdb2ae4290927beBest regards,
Signature · 1 line
Alexandre Belloni <[email protected]>