[PATCH 0/3] clk: bcm: rpi: Fixes and improvement

[PATCH 0/3] clk: bcm: rpi: Fixes and improvement

[Stefan Wahren]

This series tries to fix and improvement the Raspberry Pi firmware clock
driver. This mostly focus on clock discovery mechanism.

Just a note patch #3 depends on patch #2.

Stefan Wahren (3):
  clk: bcm: rpi: Prevent out-of-bounds access
  clk: bcm: rpi: Add missing newline
  clk: bcm: rpi: Show clock id limit in error case

 drivers/clk/bcm/clk-raspberrypi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 1/3] clk: bcm: rpi: Prevent out-of-bounds access

[Stefan Wahren]

The while loop in raspberrypi_discover_clocks() relies on the assumption
that the id of the last clock element is zero. Because this data comes
from the Videocore firmware and it doesn't guarantuee such a behavior
this could lead to out-of-bounds access. So fix this by providing
a sentinel element.

Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
Link: https://github.com/raspberrypi/firmware/issues/1688
Suggested-by: Phil Elwell <phil@raspberrypi.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 73518009a0f2..79cbf0c0b401 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -344,8 +344,13 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
 	struct rpi_firmware_get_clocks_response *clks;
 	int ret;
 
+	/*
+	 * The firmware doesn't guarantee that the last element of
+	 * RPI_FIRMWARE_GET_CLOCKS is zeroed. So allocate an additional
+	 * zero element as sentinel.
+	 */
 	clks = devm_kcalloc(rpi->dev,
-			    RPI_FIRMWARE_NUM_CLK_ID, sizeof(*clks),
+			    RPI_FIRMWARE_NUM_CLK_ID + 1, sizeof(*clks),
 			    GFP_KERNEL);
 	if (!clks)
 		return -ENOMEM;
-- 
2.25.1

[PATCH 2/3] clk: bcm: rpi: Add missing newline

[Stefan Wahren]

Some log messages lacks the final newline. So add them.

Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index 79cbf0c0b401..b3b629516182 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -220,7 +220,7 @@ static int raspberrypi_fw_set_rate(struct clk_hw *hw, unsigned long rate,
 	ret = raspberrypi_clock_property(rpi->firmware, data,
 					 RPI_FIRMWARE_SET_CLOCK_RATE, &_rate);
 	if (ret)
-		dev_err_ratelimited(rpi->dev, "Failed to change %s frequency: %d",
+		dev_err_ratelimited(rpi->dev, "Failed to change %s frequency: %d\n",
 				    clk_hw_get_name(hw), ret);
 
 	return ret;
@@ -288,7 +288,7 @@ static struct clk_hw *raspberrypi_clk_register(struct raspberrypi_clk *rpi,
 					 RPI_FIRMWARE_GET_MIN_CLOCK_RATE,
 					 &min_rate);
 	if (ret) {
-		dev_err(rpi->dev, "Failed to get clock %d min freq: %d",
+		dev_err(rpi->dev, "Failed to get clock %d min freq: %d\n",
 			id, ret);
 		return ERR_PTR(ret);
 	}
@@ -365,7 +365,7 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
 		struct raspberrypi_clk_variant *variant;
 
 		if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) {
-			dev_err(rpi->dev, "Unknown clock id: %u", clks->id);
+			dev_err(rpi->dev, "Unknown clock id: %u\n", clks->id);
 			return -EINVAL;
 		}
 
-- 
2.25.1

[PATCH 3/3] clk: bcm: rpi: Show clock id limit in error case

[Stefan Wahren]

The clock id limit will be extended in the future, so it would be
helpful to see the actual clock id limit in case the firmware
response has been rejected.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/clk/bcm/clk-raspberrypi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c
index b3b629516182..676e5379ebce 100644
--- a/drivers/clk/bcm/clk-raspberrypi.c
+++ b/drivers/clk/bcm/clk-raspberrypi.c
@@ -365,7 +365,8 @@ static int raspberrypi_discover_clocks(struct raspberrypi_clk *rpi,
 		struct raspberrypi_clk_variant *variant;
 
 		if (clks->id > RPI_FIRMWARE_NUM_CLK_ID) {
-			dev_err(rpi->dev, "Unknown clock id: %u\n", clks->id);
+			dev_err(rpi->dev, "Unknown clock id: %u (max: %u)\n",
+					   clks->id, RPI_FIRMWARE_NUM_CLK_ID);
 			return -EINVAL;
 		}
 
-- 
2.25.1

Re: [PATCH 1/3] clk: bcm: rpi: Prevent out-of-bounds access

[Florian Fainelli]

On 7/13/22 08:49, Stefan Wahren wrote:
> The while loop in raspberrypi_discover_clocks() relies on the assumption
> that the id of the last clock element is zero. Because this data comes
> from the Videocore firmware and it doesn't guarantuee such a behavior
> this could lead to out-of-bounds access. So fix this by providing
> a sentinel element.
> 
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Link: https://github.com/raspberrypi/firmware/issues/1688
> Suggested-by: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 2/3] clk: bcm: rpi: Add missing newline

[Florian Fainelli]

On 7/13/22 08:49, Stefan Wahren wrote:
> Some log messages lacks the final newline. So add them.
> 
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 3/3] clk: bcm: rpi: Show clock id limit in error case

[Florian Fainelli]

On 7/13/22 08:49, Stefan Wahren wrote:
> The clock id limit will be extended in the future, so it would be
> helpful to see the actual clock id limit in case the firmware
> response has been rejected.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

Re: [PATCH 0/3] clk: bcm: rpi: Fixes and improvement

[Ivan T. Ivanov]

Hi,

On 07-13 17:49, Stefan Wahren wrote:
> 
> This series tries to fix and improvement the Raspberry Pi firmware clock
> driver. This mostly focus on clock discovery mechanism.
> 
> Just a note patch #3 depends on patch #2.
> 
> Stefan Wahren (3):
>   clk: bcm: rpi: Prevent out-of-bounds access
>   clk: bcm: rpi: Add missing newline
>   clk: bcm: rpi: Show clock id limit in error case
> 

Maybe is little bit late, but still :-)

Reviewed-by: Ivan T. Ivanov <iivanov@suse.de>

Thank you!
Ivan

Re: [PATCH 0/3] clk: bcm: rpi: Fixes and improvement

[Florian Fainelli]

On 7/25/22 01:18, Ivan T. Ivanov wrote:
> Hi,
> 
> On 07-13 17:49, Stefan Wahren wrote:
>>
>> This series tries to fix and improvement the Raspberry Pi firmware clock
>> driver. This mostly focus on clock discovery mechanism.
>>
>> Just a note patch #3 depends on patch #2.
>>
>> Stefan Wahren (3):
>>   clk: bcm: rpi: Prevent out-of-bounds access
>>   clk: bcm: rpi: Add missing newline
>>   clk: bcm: rpi: Show clock id limit in error case
>>
> 
> Maybe is little bit late, but still :-)
> 
> Reviewed-by: Ivan T. Ivanov <iivanov@suse.de>

Stephen, can you apply those patches? Thanks! 
-- 
Florian

Re: [PATCH 0/3] clk: bcm: rpi: Fixes and improvement

[Stefan Wahren]

Hi Stephen,

Am 04.08.22 um 19:58 schrieb Florian Fainelli:
> On 7/25/22 01:18, Ivan T. Ivanov wrote:
>> Hi,
>>
>> On 07-13 17:49, Stefan Wahren wrote:
>>> This series tries to fix and improvement the Raspberry Pi firmware clock
>>> driver. This mostly focus on clock discovery mechanism.
>>>
>>> Just a note patch #3 depends on patch #2.
>>>
>>> Stefan Wahren (3):
>>>    clk: bcm: rpi: Prevent out-of-bounds access
>>>    clk: bcm: rpi: Add missing newline
>>>    clk: bcm: rpi: Show clock id limit in error case
>>>
>> Maybe is little bit late, but still :-)
>>
>> Reviewed-by: Ivan T. Ivanov <iivanov@suse.de>
> Stephen, can you apply those patches? Thanks!

should i resend this series?

Best regards

Re: [PATCH 3/3] clk: bcm: rpi: Show clock id limit in error case

[Stephen Boyd]

Quoting Stefan Wahren (2022-07-13 08:49:53)
> The clock id limit will be extended in the future, so it would be
> helpful to see the actual clock id limit in case the firmware
> response has been rejected.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---

Applied to clk-fixes

Re: [PATCH 2/3] clk: bcm: rpi: Add missing newline

[Stephen Boyd]

Quoting Stefan Wahren (2022-07-13 08:49:52)
> Some log messages lacks the final newline. So add them.
> 
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---

Applied to clk-fixes

Re: [PATCH 1/3] clk: bcm: rpi: Prevent out-of-bounds access

[Stephen Boyd]

Quoting Stefan Wahren (2022-07-13 08:49:51)
> The while loop in raspberrypi_discover_clocks() relies on the assumption
> that the id of the last clock element is zero. Because this data comes
> from the Videocore firmware and it doesn't guarantuee such a behavior
> this could lead to out-of-bounds access. So fix this by providing
> a sentinel element.
> 
> Fixes: 93d2725affd6 ("clk: bcm: rpi: Discover the firmware clocks")
> Link: https://github.com/raspberrypi/firmware/issues/1688
> Suggested-by: Phil Elwell <phil@raspberrypi.com>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---

Applied to clk-fixes