[PATCH] clk: zynq: eval ps-clock-frequency from DT

[PATCH] clk: zynq: eval ps-clock-frequency from DT

[Steffen Trumtrar]

From: Kai Assman <kai.assmann@de.bosch.com>

Currently the ps_clk_rate is locked to 33.3MHz. The devicetree
provides a property "ps-clock-frequency" that specifies this clock.

If the property is found, overwrite ps_clk_rate otherwise stay at the
default 33.3MHz

Signed-off-by: Kai Assmann <kai.assmann@de.bosch.com>
Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/clk/zynq/clkc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
index 8e4beda295..37a0fbadb5 100644
--- a/drivers/clk/zynq/clkc.c
+++ b/drivers/clk/zynq/clkc.c
@@ -388,6 +388,9 @@ static int zynq_clock_probe(struct device *dev)
 			return PTR_ERR(parent_res);
 
 		slcr_offset = parent_res->start;
+
+		of_property_read_u32(dev->device_node, "ps-clock-frequency",
+				     (u32 *)&ps_clk_rate);
 	}
 
 	iores = request_iomem_region(dev_name(dev), iores->start + slcr_offset,
-- 
2.39.0

Re: [PATCH] clk: zynq: eval ps-clock-frequency from DT

[Ahmad Fatoum]

On 08.05.23 09:24, Steffen Trumtrar wrote:
> From: Kai Assman <kai.assmann@de.bosch.com>
> 
> Currently the ps_clk_rate is locked to 33.3MHz. The devicetree
> provides a property "ps-clock-frequency" that specifies this clock.
> 
> If the property is found, overwrite ps_clk_rate otherwise stay at the
> default 33.3MHz

The commit message reads as is this is an upstream binding, but I find no
usage, documentation or driver code parsing it in Linux v6.4-rc1.

Is this a new barebox-specific binding? If so, the commit message should be
reworded to make this clear. You can also drop a short .rst file into
Documentation/devicetree/bindings/clocks to describe your new property.

> Signed-off-by: Kai Assmann <kai.assmann@de.bosch.com>
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> ---
>  drivers/clk/zynq/clkc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
> index 8e4beda295..37a0fbadb5 100644
> --- a/drivers/clk/zynq/clkc.c
> +++ b/drivers/clk/zynq/clkc.c
> @@ -388,6 +388,9 @@ static int zynq_clock_probe(struct device *dev)
>  			return PTR_ERR(parent_res);
>  
>  		slcr_offset = parent_res->start;
> +
> +		of_property_read_u32(dev->device_node, "ps-clock-frequency",
> +				     (u32 *)&ps_clk_rate);

Please do not cast compiler warnings away. You should change the type of
ps_clk_rate instead to u32, so this code is 64-bit-safe.

Cheers,
Ahmad

>  	}
>  
>  	iores = request_iomem_region(dev_name(dev), iores->start + slcr_offset,

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] clk: zynq: eval ps-clock-frequency from DT

[Steffen Trumtrar]

On 2023-05-08 at 13:17 +02, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> On 08.05.23 09:24, Steffen Trumtrar wrote:
>> From: Kai Assman <kai.assmann@de.bosch.com>
>>
>> Currently the ps_clk_rate is locked to 33.3MHz. The devicetree
>> provides a property "ps-clock-frequency" that specifies this clock.
>>
>> If the property is found, overwrite ps_clk_rate otherwise stay at the
>> default 33.3MHz
>
> The commit message reads as is this is an upstream binding, but I find no
> usage, documentation or driver code parsing it in Linux v6.4-rc1.
>
> Is this a new barebox-specific binding? If so, the commit message should be
> reworded to make this clear. You can also drop a short .rst file into
> Documentation/devicetree/bindings/clocks to describe your new property.
>

Ha, you are right. I could have sworn to have seen this binding already
and didn't check :(

I'll change that.

>> Signed-off-by: Kai Assmann <kai.assmann@de.bosch.com>
>> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
>> ---
>>  drivers/clk/zynq/clkc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/clk/zynq/clkc.c b/drivers/clk/zynq/clkc.c
>> index 8e4beda295..37a0fbadb5 100644
>> --- a/drivers/clk/zynq/clkc.c
>> +++ b/drivers/clk/zynq/clkc.c
>> @@ -388,6 +388,9 @@ static int zynq_clock_probe(struct device *dev)
>>  			return PTR_ERR(parent_res);
>>
>>  		slcr_offset = parent_res->start;
>> +
>> +		of_property_read_u32(dev->device_node, "ps-clock-frequency",
>> +				     (u32 *)&ps_clk_rate);
>
> Please do not cast compiler warnings away. You should change the type of
> ps_clk_rate instead to u32, so this code is 64-bit-safe.

Ack.


Thanks,
Steffen

--
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |