Messages in this thread | | | Date | Mon, 14 Mar 2022 11:43:26 +0000 | Subject | Re: [PATCH v2 3/3] ARM: dts: lpc32xx: Update spi clock properties | From | Robin Murphy <> |
| |
On 2022-03-11 14:07, Vladimir Zapolskiy wrote: > On 3/11/22 3:38 PM, Arnd Bergmann wrote: >> On Fri, Mar 11, 2022 at 2:20 PM Vladimir Zapolskiy <vz@mleia.com> wrote: >>> >>> On 3/11/22 11:38 AM, Kuldeep Singh wrote: >>>> PL022 binding require two clocks to be defined but lpc platform doesn't >>>> comply with bindings and define only one clock i.e apb_pclk. >>>> >>>> Update spi clocks and clocks-names property by adding appropriate clock >>>> reference to make it compliant with bindings. >>>> >>>> CC: Vladimir Zapolskiy <vz@mleia.com> >>>> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com> >>>> --- >>>> v2: >>>> - New patch with similar changeset >>>> - Send to soc ML >>>> >>>> arch/arm/boot/dts/lpc32xx.dtsi | 8 ++++---- >>>> 1 file changed, 4 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/dts/lpc32xx.dtsi >>>> b/arch/arm/boot/dts/lpc32xx.dtsi >>>> index c87066d6c995..30958e02d5e2 100644 >>>> --- a/arch/arm/boot/dts/lpc32xx.dtsi >>>> +++ b/arch/arm/boot/dts/lpc32xx.dtsi >>>> @@ -178,8 +178,8 @@ ssp0: spi@20084000 { >>>> compatible = "arm,pl022", >>>> "arm,primecell"; >>>> reg = <0x20084000 0x1000>; >>>> interrupts = <20 IRQ_TYPE_LEVEL_HIGH>; >>>> - clocks = <&clk LPC32XX_CLK_SSP0>; >>>> - clock-names = "apb_pclk"; >>>> + clocks = <&clk LPC32XX_CLK_SSP0>, >>>> <&clk LPC32XX_CLK_SSP0>; >>>> + clock-names = "sspclk", "apb_pclk"; >>> >>> In fact I'm uncertain if it is the right change, could it happen that >>> the commit >>> cc0f6e96c4fd ("spi: dt-bindings: Convert Arm pl022 to json-schema") >>> sets a wrong >>> schema pattern? >> >> Good pointm this doesn't quite seem right: it is unlikely that the >> same clock >> is used for both the SPI bus and the APB bus. >> >>> Apparently just one clock is wanted on all observed platforms and >>> cases, this >>> is implicitly confirmed by clock handling in the >>> drivers/spi/spi-pl022.c : >>> >>> pl022->clk = devm_clk_get(&adev->dev, NULL); >>> >>> So, I would vote to fix the device tree bindings schema. >> >> Isn't this just using the wrong name? The name of the macro >> LPC32XX_CLK_SSP0 might indicate that this is indeed the SPI clock >> rather than the APB clock, so we only need to change clock-names >> property here and leave it unchanged otherwise. > > Yes, the name is wrong, here I'm ready to take the blame: > > Fixes: 93898eb775e5 ("arm: dts: lpc32xx: add clock properties to device > nodes") > > Noteworthy the commit above presets the same clock name to other PrimeCell > controllers, namely pl110 (LCD), pl080 (DMA), pl175 (EMC) and pl18x (SD), > plus this one pl022 (SSP), and all but SSP and SD are AHB slaves in fact. > > On LPC32xx the bus clock source and function clock source for SSP is HCLK. > > My guess is that the misnamed "apb_pclk" migrated into the schema from > the lpc32xx.dtsi, so I'd suggest, unless some platform really needs it, > firstly fix the schema by removing "apb_pclk" clock. It will leave just one > clock, so "clock-names" property can be set as optional, and the drop > the property from the lpc32xx.dtsi.
No, "apb_pclk" is part of the common AMBA binding, and is required by the "arm,primecell" compatible. You won't (usually) find it referenced in drivers because it's dealt with by amba_get_enable_pclk() via amba_probe().
Robin.
| |