* [PATCH v2 2/3] rtc: ds1307: Make use of device properties
2020-11-16 14:28 [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Andy Shevchenko
@ 2020-11-16 14:28 ` Andy Shevchenko
2020-11-16 14:28 ` [PATCH v2 3/3] rtc: ds1307: Drop of_match_ptr and CONFIG_OF protections Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2020-11-16 14:28 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-acpi
Cc: Andy Shevchenko
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.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: moved ds3231_clks_names to be global and static
drivers/rtc/rtc-ds1307.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index fcb8e281abd5..49260bc260e3 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -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:
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 3/3] rtc: ds1307: Drop of_match_ptr and CONFIG_OF protections
2020-11-16 14:28 [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Andy Shevchenko
2020-11-16 14:28 ` [PATCH v2 2/3] rtc: ds1307: Make use of device properties Andy Shevchenko
@ 2020-11-16 14:28 ` Andy Shevchenko
2020-11-16 18:19 ` [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Rafael J. Wysocki
2020-11-17 19:29 ` Alexandre Belloni
3 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2020-11-16 14:28 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-acpi
Cc: Andy Shevchenko
These 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.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: no changes
drivers/rtc/rtc-ds1307.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 49260bc260e3..fdf25db1b1b3 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -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_OF
static 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,
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs
2020-11-16 14:28 [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Andy Shevchenko
2020-11-16 14:28 ` [PATCH v2 2/3] rtc: ds1307: Make use of device properties Andy Shevchenko
2020-11-16 14:28 ` [PATCH v2 3/3] rtc: ds1307: Drop of_match_ptr and CONFIG_OF protections Andy Shevchenko
@ 2020-11-16 18:19 ` Rafael J. Wysocki
2020-11-17 19:29 ` Alexandre Belloni
3 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2020-11-16 18:19 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc,
ACPI Devel Maling List, Tin Huynh
On Mon, Nov 16, 2020 at 3:29 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> 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.
>
> Link: https://uefi.org/PNP_ACPI_Registry
> Cc: Tin Huynh <tnhuynh@apm.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
FWIW, the series looks good to me.
Please feel free to add
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
to the patches.
> ---
> v2: reworded commit message to clarify source of properties (Rafael)
> drivers/rtc/rtc-ds1307.c | 36 +-----------------------------------
> 1 file changed, 1 insertion(+), 35 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 9f5f54ca039d..fcb8e281abd5 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs
2020-11-16 14:28 [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Andy Shevchenko
` (2 preceding siblings ...)
2020-11-16 18:19 ` [PATCH v2 1/3] rtc: ds1307: Remove non-valid ACPI IDs Rafael J. Wysocki
@ 2020-11-17 19:29 ` Alexandre Belloni
3 siblings, 0 replies; 5+ messages in thread
From: Alexandre Belloni @ 2020-11-17 19:29 UTC (permalink / raw)
To: linux-acpi, Alessandro Zummo, Andy Shevchenko, linux-rtc
Cc: Alexandre Belloni, Tin Huynh
On Mon, 16 Nov 2020 16:28:57 +0200, Andy Shevchenko wrote:
> 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: 698fffc2705cc48804cc31021cdb2ae4290927be
Best regards,
--
Alexandre Belloni <alexandre.belloni@bootlin.com>
^ permalink raw reply [flat|nested] 5+ messages in thread