Re: [PATCH v4 0/3] clk: bcm: rpi: Add support for three more clocks

From: Stefan Wahren <stefan.wahren@i2se.com>
To: "Ivan T. Ivanov" <iivanov@suse.de>
Cc: Maxime Ripard <maxime@cerno.tech>,
	Guillaume Gardet <Guillaume.Gardet@arm.com>,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	bcm-kernel-feedback-list@broadcom.com, linux-clk@vger.kernel.org,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org, nd <nd@arm.com>
Subject: Re: [PATCH v4 0/3] clk: bcm: rpi: Add support for three more clocks
Date: Mon, 11 Jul 2022 13:55:36 +0200	[thread overview]
Message-ID: <c131ad48-aaea-7d56-6370-5131c11251d6@i2se.com> (raw)
In-Reply-To: <b5c560cf5610fa8da1cfd7ef06ca6bac@suse.de>

Hi,

Am 11.07.22 um 09:02 schrieb Ivan T. Ivanov:
>
> Hi,
>
> On 2022-07-10 14:12, Stefan Wahren wrote:
>> Hi,
>>
>> Am 10.05.22 um 15:30 schrieb Maxime Ripard:
>>> Hi,
>>>
>>> On Tue, May 10, 2022 at 01:20:18PM +0000, Guillaume Gardet wrote:
>>>> May I ask what's the status/plan of this patch series?
>>> As far as I know it hasn't been merged yet.
>>>
>>>> It seems it has not been merged yet, and I know we are a bit late in
>>>> the 5.18 schedule, but I think this is a good fix for 5.18.
>>> Fix for what? I don't think this series fix any bug?
>>
>> i think this series (or at least parts of it) is a workaround for this
>> issue [1].
>
> No, not really. Bug which this patch set fixes is this one [2].

Both are related. In your case the device tree defines a clock ID which 
isn't define in the kernel driver. So the behavior (abort probing in 
case of invalid DTB) is correct to me. But the log message isn't helpful 
to end users.

This is broken by design, since we need to synchronize videocore 
firmware, kernel and DTB at the same time. Maybe this works for 
Raspberry Pi OS, but not for all users.

We need to fix this driver before add new clocks.

> When using downstream dtb and up to date bootloader index 15 is
> valid id RPI_FIRMWARE_VEC_CLK_ID which is used by vc4 module.
I think this only applies to the downstream kernel. AFAIK the 
bcm2835-clk is still used for VEC in mainline.
>
>>
>> We better fix the root cause of the potential out of bounds access in
>> clk-raspberrypi properly. I will send a patch soon.
>
> If not mistaken after last rework driver already handle this properly.
> clk-raspberrypi.c:362

No, this doesn't prevent the possible out of bounds access, what i mean. 
It's the condition of the while loop.

Best regards

>
> Regards,
> Ivan
>
>>
>> [1] - https://github.com/raspberrypi/firmware/issues/1688
>
> [2] https://bugzilla.suse.com/show_bug.cgi?id=1196632
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel