netdev - Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <98ec373b-7c7d-4775-b166-9be1e1b3eda1@kernel.org>
Date: Thu, 22 Aug 2024 16:46:07 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Andrea della Porta <andrea.porta@...e.com>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Florian Fainelli <florian.fainelli@...adcom.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
 Linus Walleij <linus.walleij@...aro.org>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Derek Kiernan <derek.kiernan@....com>, Dragan Cvetic
 <dragan.cvetic@....com>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Saravana Kannan <saravanak@...gle.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
 netdev@...r.kernel.org, linux-pci@...r.kernel.org,
 linux-arch@...r.kernel.org, Lee Jones <lee@...nel.org>,
 Andrew Lunn <andrew@...n.ch>, Stefan Wahren <wahrenst@....net>
Subject: Re: [PATCH 08/11] misc: rp1: RaspberryPi RP1 misc driver

On 22/08/2024 16:33, Andrea della Porta wrote:
> Hi Krzysztof,
> 
> On 16:20 Wed 21 Aug     , Krzysztof Kozlowski wrote:
>> On 21/08/2024 10:38, Krzysztof Kozlowski wrote:
>>> On Tue, Aug 20, 2024 at 04:36:10PM +0200, Andrea della Porta wrote:
>>
>> ...
>>
>>>>  drivers/misc/Kconfig                  |   1 +
>>>>  drivers/misc/Makefile                 |   1 +
>>>>  drivers/misc/rp1/Kconfig              |  20 ++
>>>>  drivers/misc/rp1/Makefile             |   3 +
>>>>  drivers/misc/rp1/rp1-pci.c            | 333 ++++++++++++++++++++++++++
>>>>  drivers/misc/rp1/rp1-pci.dtso         |   8 +
>>>>  drivers/pci/quirks.c                  |   1 +
>>>>  include/linux/pci_ids.h               |   3 +
>>>>  10 files changed, 524 insertions(+)
>>>>  create mode 100644 arch/arm64/boot/dts/broadcom/rp1.dtso
>>>>  create mode 100644 drivers/misc/rp1/Kconfig
>>>>  create mode 100644 drivers/misc/rp1/Makefile
>>>>  create mode 100644 drivers/misc/rp1/rp1-pci.c
>>>>  create mode 100644 drivers/misc/rp1/rp1-pci.dtso
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 67f460c36ea1..1359538b76e8 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -19119,9 +19119,11 @@ F:	include/uapi/linux/media/raspberrypi/
>>>>  RASPBERRY PI RP1 PCI DRIVER
>>>>  M:	Andrea della Porta <andrea.porta@...e.com>
>>>>  S:	Maintained
>>>> +F:	arch/arm64/boot/dts/broadcom/rp1.dtso
>>>>  F:	Documentation/devicetree/bindings/clock/raspberrypi,rp1-clocks.yaml
>>>>  F:	Documentation/devicetree/bindings/pinctrl/raspberrypi,rp1-gpio.yaml
>>>>  F:	drivers/clk/clk-rp1.c
>>>> +F:	drivers/misc/rp1/
>>>>  F:	drivers/pinctrl/pinctrl-rp1.c
>>>>  F:	include/dt-bindings/clock/rp1.h
>>>>  F:	include/dt-bindings/misc/rp1.h
>>>> diff --git a/arch/arm64/boot/dts/broadcom/rp1.dtso b/arch/arm64/boot/dts/broadcom/rp1.dtso
>>>> new file mode 100644
>>>> index 000000000000..d80178a278ee
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/broadcom/rp1.dtso
>>>> @@ -0,0 +1,152 @@
>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>> +
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +#include <dt-bindings/interrupt-controller/irq.h>
>>>> +#include <dt-bindings/clock/rp1.h>
>>>> +#include <dt-bindings/misc/rp1.h>
>>>> +
>>>> +/dts-v1/;
>>>> +/plugin/;
>>>> +
>>>> +/ {
>>>> +	fragment@0 {
>>>> +		target-path="";
>>>> +		__overlay__ {
>>>> +			#address-cells = <3>;
>>>> +			#size-cells = <2>;
>>>> +
>>>> +			rp1: rp1@0 {
>>>> +				compatible = "simple-bus";
>>>> +				#address-cells = <2>;
>>>> +				#size-cells = <2>;
>>>> +				interrupt-controller;
>>>> +				interrupt-parent = <&rp1>;
>>>> +				#interrupt-cells = <2>;
>>>> +
>>>> +				// ranges and dma-ranges must be provided by the includer
>>>> +				ranges = <0xc0 0x40000000
>>>> +					  0x01/*0x02000000*/ 0x00 0x00000000
>>>> +					  0x00 0x00400000>;
>>>
>>> Are you 100% sure you do not have here dtc W=1 warnings?
>>
>> One more thing, I do not see this overlay applied to any target, which
>> means it cannot be tested. You miss entry in Makefile.
>>
> 
> The dtso is intended to be built from driver/misc/rp1/Makefile as it will
> be included in the driver obj:
> 
> --- /dev/null
> +++ b/drivers/misc/rp1/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +rp1-pci-objs                   := rp1-pci.o rp1-pci.dtbo.o
> +obj-$(CONFIG_MISC_RP1)         += rp1-pci.o
> 
> and not as part of the dtb system, hence it's m issing in
> arch/arm64/boot/dts/broadcom/Makefile.
> 
> On the other hand:
> 
> #> make W=1 CHECK_DTBS=y broadcom/rp1.dtbo
>   DTC     arch/arm64/boot/dts/broadcom/rp1.dtbo
> arch/arm64/boot/dts/broadcom/rp1.dtso:37.24-42.7: Warning (simple_bus_reg): /fragment@..._overlay__/rp1@...lk_xosc: missing or empty reg/ranges property
> arch/arm64/boot/dts/broadcom/rp1.dtso:44.26-49.7: Warning (simple_bus_reg): /fragment@..._overlay__/rp1@...acb_pclk: missing or empty reg/ranges property
> arch/arm64/boot/dts/broadcom/rp1.dtso:51.26-56.7: Warning (simple_bus_reg): /fragment@..._overlay__/rp1@...acb_hclk: missing or empty reg/ranges property
> arch/arm64/boot/dts/broadcom/rp1.dtso:14.15-173.5: Warning (avoid_unnecessary_addr_size): /fragment@..._overlay__: unnecessary #address-cells/#size-cells without "ranges", "dma-ranges" or child "reg" property
> 
> seems to do the checks, unless I'm missing something.

Yeah, there is still no target which applies the overlay, so no one can
tell whether it applies cleanly or not. You can only test single
overlay, but it is expected to test each overlay being applied to chosen
DTB.

Best regards,
Krzysztof