| Message ID | 20240731062814.215833-2-iivanov@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add pin control driver for BCM2712 SoC | expand |
On 31/07/2024 08:28, Ivan T. Ivanov wrote: > It looks like they are few revisions of this chip which varies > by number of pins. Perhaps not all of them are available on the > market. Perhaps some of them where early engineering samples, > I don't know. I decided to keep all of them just in case. > > Cc: Andrea della Porta <andrea.porta@suse.com> > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de> > --- > .../pinctrl/brcm,brcmstb-pinctrl.yaml | 73 +++++++++++++++++++ > 1 file changed, 73 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > new file mode 100644 > index 000000000000..c5afdb49d784 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml bcm2712 is Rpi, so not really STB. The name is confusing. Please use compatible as filename, so: brcm,bcm2712-pinctrl.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom STB family pin controller > + > +maintainers: > + - Ivan T. Ivanov <iivanov@suse.de> > + > +description: > + Broadcom's STB family memory-mapped pin controller. > + > +properties: > + compatible: > + enum: > + - brcm,bcm2712-pinctrl > + - brcm,bcm2712-aon-pinctrl > + - brcm,bcm2712c0-pinctrl > + - brcm,bcm2712c0-aon-pinctrl > + - brcm,bcm2712d0-pinctrl > + - brcm,bcm2712d0-aon-pinctrl > + > + reg: > + maxItems: 1 > + > +allOf: > + - $ref: pinctrl.yaml# "allOf:" block goes after "required:". > + > +required: > + - compatible > + - reg > + > +additionalProperties: > + anyOf: > + - type: object > + allOf: > + - $ref: pincfg-node.yaml# > + - $ref: pinmux-node.yaml# > + > + properties: > + function: > + enum: > + [ Unnecessary new line > + gpio, alt1, alt2, alt3, alt4, alt5, alt6, alt7, alt8, > + aon_cpu_standbyb, aon_fp_4sec_resetb, aon_gpclk, aon_pwm, > + arm_jtag, aud_fs_clk0, avs_pmu_bsc, bsc_m0, bsc_m1, bsc_m2, > + bsc_m3, clk_observe, ctl_hdmi_5v, enet0, enet0_mii, enet0_rgmii, > + ext_sc_clk, fl0, fl1, gpclk0, gpclk1, gpclk2, hdmi_tx0_auto_i2c, > + hdmi_tx0_bsc, hdmi_tx1_auto_i2c, hdmi_tx1_bsc, i2s_in, i2s_out, > + ir_in, mtsif, mtsif_alt, mtsif_alt1, pdm, pkt, pm_led_out, sc0, > + sd0, sd2, sd_card_a, sd_card_b, sd_card_c, sd_card_d, sd_card_e, > + sd_card_f, sd_card_g, spdif_out, spi_m, spi_s, sr_edm_sense, te0, > + te1, tsio, uart0, uart1, uart2, usb_pwr, usb_vbus, uui, vc_i2c0, > + vc_i2c3, vc_i2c4, vc_i2c5, vc_i2csl, vc_pcm, vc_pwm0, vc_pwm1, > + vc_spi0, vc_spi3, vc_spi4, vc_spi5, vc_uart0, vc_uart2, vc_uart3, > + vc_uart4, > + ] > + > + pins: > + items: > + pattern: "^((aon_)?s?gpio[0-6]?[0-9])|(emmc_(clk|cmd|dat[0-7]|ds))$" > + > + bias-disable: true > + bias-pull-down: true > + bias-pull-up: true > + additionalProperties: false > + > + - type: object > + additionalProperties: > + $ref: "#/additionalProperties/anyOf/0" I suggest going with patternProperties, fixed suffix for node names and $defs. See for example: Documentation/devicetree/bindings/pinctrl/qcom,x1e80100-tlmm.yaml Missing example. I don't see this being part of other complete device, so example is a requirement. Best regards, Krzysztof
Hi, On 08-01 10:17, Krzysztof Kozlowski wrote: > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > bcm2712 is Rpi, so not really STB. The name is confusing. Please use > compatible as filename, so: > brcm,bcm2712-pinctrl.yaml According Florian it is: https://lore.kernel.org/lkml/f6601f73-cb22-4ba3-88c5-241be8421fc3@broadcom.com/ > > > @@ -0,0 +1,73 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Broadcom STB family pin controller > > + > > +maintainers: > > + - Ivan T. Ivanov <iivanov@suse.de> > > + > > +description: > > + Broadcom's STB family memory-mapped pin controller. > > + > > +properties: > > + compatible: > > + enum: > > + - brcm,bcm2712-pinctrl > > + - brcm,bcm2712-aon-pinctrl > > + - brcm,bcm2712c0-pinctrl > > + - brcm,bcm2712c0-aon-pinctrl > > + - brcm,bcm2712d0-pinctrl > > + - brcm,bcm2712d0-aon-pinctrl > > + > > + reg: > > + maxItems: 1 > > + > > +allOf: > > + - $ref: pinctrl.yaml# > > "allOf:" block goes after "required:". > > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: > > + anyOf: > > + - type: object > > + allOf: > > + - $ref: pincfg-node.yaml# > > + - $ref: pinmux-node.yaml# > > + > > + properties: > > + function: > > + enum: > > + [ > > Unnecessary new line > > > + gpio, alt1, alt2, alt3, alt4, alt5, alt6, alt7, alt8, > > + aon_cpu_standbyb, aon_fp_4sec_resetb, aon_gpclk, aon_pwm, > > + arm_jtag, aud_fs_clk0, avs_pmu_bsc, bsc_m0, bsc_m1, bsc_m2, > > + bsc_m3, clk_observe, ctl_hdmi_5v, enet0, enet0_mii, enet0_rgmii, > > + ext_sc_clk, fl0, fl1, gpclk0, gpclk1, gpclk2, hdmi_tx0_auto_i2c, > > + hdmi_tx0_bsc, hdmi_tx1_auto_i2c, hdmi_tx1_bsc, i2s_in, i2s_out, > > + ir_in, mtsif, mtsif_alt, mtsif_alt1, pdm, pkt, pm_led_out, sc0, > > + sd0, sd2, sd_card_a, sd_card_b, sd_card_c, sd_card_d, sd_card_e, > > + sd_card_f, sd_card_g, spdif_out, spi_m, spi_s, sr_edm_sense, te0, > > + te1, tsio, uart0, uart1, uart2, usb_pwr, usb_vbus, uui, vc_i2c0, > > + vc_i2c3, vc_i2c4, vc_i2c5, vc_i2csl, vc_pcm, vc_pwm0, vc_pwm1, > > + vc_spi0, vc_spi3, vc_spi4, vc_spi5, vc_uart0, vc_uart2, vc_uart3, > > + vc_uart4, > > + ] > > + > > + pins: > > + items: > > + pattern: "^((aon_)?s?gpio[0-6]?[0-9])|(emmc_(clk|cmd|dat[0-7]|ds))$" > > + > > + bias-disable: true > > + bias-pull-down: true > > + bias-pull-up: true > > + additionalProperties: false > > + > > + - type: object > > + additionalProperties: > > + $ref: "#/additionalProperties/anyOf/0" > > I suggest going with patternProperties, fixed suffix for node names and > $defs. See for example: > Documentation/devicetree/bindings/pinctrl/qcom,x1e80100-tlmm.yaml > > Missing example. I don't see this being part of other complete device, > so example is a requirement. > Thanks, I will update and resend. Regards, Ivan
On 01/08/2024 10:38, Ivan T. Ivanov wrote: > Hi, > > On 08-01 10:17, Krzysztof Kozlowski wrote: >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml >> >> bcm2712 is Rpi, so not really STB. The name is confusing. Please use >> compatible as filename, so: >> brcm,bcm2712-pinctrl.yaml > > According Florian it is: > > https://lore.kernel.org/lkml/f6601f73-cb22-4ba3-88c5-241be8421fc3@broadcom.com/ OK, title can be like this, no problem, although then please expand what "STB" means. Bindings still are supposed to use compatible as filename. Best regards, Krzysztof
Hi, [add official Raspberry Pi kernel developer list] Am 31.07.24 um 08:28 schrieb Ivan T. Ivanov: > It looks like they are few revisions of this chip which varies > by number of pins. Perhaps not all of them are available on the > market. Perhaps some of them where early engineering samples, > I don't know. I decided to keep all of them just in case. The BCM2711 had also some revisions and we avoided successfully multiple versions of the RPi 4B DTS. So it would be nice if someone can explain if C0 & D0 are available in the market? Otherwise we may end up with multiple versions of the RPi 5 DTS. I'm missing an explanation in the commit message, what's the difference between brcm,bcm2712-pinctrl and brcm,bcm2712-aon-pinctrl? According to the driver brcm,bcm2712-pinctrl is the same as brcm,bcm2712c0-pinctrl. So the former is more a fallback? Thanks > > Cc: Andrea della Porta <andrea.porta@suse.com> > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de> > --- > .../pinctrl/brcm,brcmstb-pinctrl.yaml | 73 +++++++++++++++++++ > 1 file changed, 73 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > new file mode 100644 > index 000000000000..c5afdb49d784 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom STB family pin controller > + > +maintainers: > + - Ivan T. Ivanov <iivanov@suse.de> > + > +description: > + Broadcom's STB family memory-mapped pin controller. > + > +properties: > + compatible: > + enum: > + - brcm,bcm2712-pinctrl > + - brcm,bcm2712-aon-pinctrl > + - brcm,bcm2712c0-pinctrl > + - brcm,bcm2712c0-aon-pinctrl > + - brcm,bcm2712d0-pinctrl > + - brcm,bcm2712d0-aon-pinctrl > + >
Hi, On 08-01 16:19, Krzysztof Kozlowski wrote: > > On 01/08/2024 10:38, Ivan T. Ivanov wrote: > > Hi, > > > > On 08-01 10:17, Krzysztof Kozlowski wrote: > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > >> > >> bcm2712 is Rpi, so not really STB. The name is confusing. Please use > >> compatible as filename, so: > >> brcm,bcm2712-pinctrl.yaml > > > > According Florian it is: > > > > https://lore.kernel.org/lkml/f6601f73-cb22-4ba3-88c5-241be8421fc3@broadcom.com/ > > OK, title can be like this, no problem, although then please expand what > "STB" means. Ok. > > Bindings still are supposed to use compatible as filename. > IIUC, you suggest to rename compatible string from brcm,bcm2712-pinctrl to brcm,brcmstb-pinctrl?! Regards, Ivan
Hi Stefan Sorry for the delay in responding - I was on holiday last week. On Fri, 2 Aug 2024 at 19:10, Stefan Wahren <wahrenst@gmx.net> wrote: > > Hi, > > [add official Raspberry Pi kernel developer list] > > Am 31.07.24 um 08:28 schrieb Ivan T. Ivanov: > > It looks like they are few revisions of this chip which varies > > by number of pins. Perhaps not all of them are available on the > > market. Perhaps some of them where early engineering samples, > > I don't know. I decided to keep all of them just in case. > The BCM2711 had also some revisions and we avoided successfully multiple > versions of the RPi 4B DTS. So it would be nice if someone can explain > if C0 & D0 are available in the market? Otherwise we may end up with > multiple versions of the RPi 5 DTS. AFAIK A0 and B0 silicon were never commercialised. C0 is the current revision in use with Pi5. D0 will be in devices imminently. CM5 will use it from launch, but subsequently standard Pi5s will do as well. In addition to putting in the few fixes that were desired, some registers and DMA dreqs got shuffled around, hence some drivers will need additional compatible strings (vc4 certainly does) and other minor DT tweaks. Checking our downstream dt files, we have bcm2712d0-rpi-5-b.dts[1] that includes and patches the original (C0) bcm2712-rpi-5-b.dts[2]. The cleaner option would be to have a common bcm2712-rpi-5-b.dts(i) and separate bcm2712c0-rpi-5-b.dts bcm2712d0-rpi-5-b.dts which include the base and add the relevant customisations. Later a bcm2712d0-rpi-cm5.dts DT should be able to include that same base file as well. I'm not quite sure why the GPIO names are redefined in our d0 file - other than the unused ones using "-" instead of "", they appear identical. > I'm missing an explanation in the commit message, what's the difference > between brcm,bcm2712-pinctrl and brcm,bcm2712-aon-pinctrl? Two separate instantiations of the same IP block, but they differ in the number of pins that are associated and the pinmux functions for each of those pins. AFAIK there is no way from DT to specify those pinmux function names, so otherwise /sys/kernel/debug/pinctrl/<node>/pins will give the wrong function mappings. > According to the driver brcm,bcm2712-pinctrl is the same as > brcm,bcm2712c0-pinctrl. So the former is more a fallback? I'd need to check with Phil (who's on holiday this week) or Dom, but I believe you are correct that "brcm,bcm2712-pinctrl" is a fallback. Most likely due to our early DT files not having the c0 designation. Obviously for mainline that is irrelevant, so dropping the non-specific compatibles is fine. I hope that makes some more sense. Dave [1] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/bcm2712d0-rpi-5-b.dts [2] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts > Thanks > > > > Cc: Andrea della Porta <andrea.porta@suse.com> > > Signed-off-by: Ivan T. Ivanov <iivanov@suse.de> > > --- > > .../pinctrl/brcm,brcmstb-pinctrl.yaml | 73 +++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > new file mode 100644 > > index 000000000000..c5afdb49d784 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml > > @@ -0,0 +1,73 @@ > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Broadcom STB family pin controller > > + > > +maintainers: > > + - Ivan T. Ivanov <iivanov@suse.de> > > + > > +description: > > + Broadcom's STB family memory-mapped pin controller. > > + > > +properties: > > + compatible: > > + enum: > > + - brcm,bcm2712-pinctrl > > + - brcm,bcm2712-aon-pinctrl > > + - brcm,bcm2712c0-pinctrl > > + - brcm,bcm2712c0-aon-pinctrl > > + - brcm,bcm2712d0-pinctrl > > + - brcm,bcm2712d0-aon-pinctrl > > + > > >
Hi Dave, Am 12.08.24 um 18:28 schrieb Dave Stevenson: > Hi Stefan > > Sorry for the delay in responding - I was on holiday last week. > no problem and thanks for the explanations. >> I'm missing an explanation in the commit message, what's the difference >> between brcm,bcm2712-pinctrl and brcm,bcm2712-aon-pinctrl? > Two separate instantiations of the same IP block, but they differ in > the number of pins that are associated and the pinmux functions for > each of those pins. AFAIK there is no way from DT to specify those > pinmux function names, so otherwise > /sys/kernel/debug/pinctrl/<node>/pins will give the wrong function > mappings. Yes, my request is that this or a similar explanation should go to the commit message, because not all reviewers know the IP block. It would be great to explain the aon part. Always on? >> According to the driver brcm,bcm2712-pinctrl is the same as >> brcm,bcm2712c0-pinctrl. So the former is more a fallback? > I'd need to check with Phil (who's on holiday this week) or Dom, but I > believe you are correct that "brcm,bcm2712-pinctrl" is a fallback. > Most likely due to our early DT files not having the c0 designation. > Obviously for mainline that is irrelevant, so dropping the > non-specific compatibles is fine. I agree. Regards > > I hope that makes some more sense. > > Dave > > [1] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/bcm2712d0-rpi-5-b.dts > [2] https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml new file mode 100644 index 000000000000..c5afdb49d784 --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom STB family pin controller + +maintainers: + - Ivan T. Ivanov <iivanov@suse.de> + +description: + Broadcom's STB family memory-mapped pin controller. + +properties: + compatible: + enum: + - brcm,bcm2712-pinctrl + - brcm,bcm2712-aon-pinctrl + - brcm,bcm2712c0-pinctrl + - brcm,bcm2712c0-aon-pinctrl + - brcm,bcm2712d0-pinctrl + - brcm,bcm2712d0-aon-pinctrl + + reg: + maxItems: 1 + +allOf: + - $ref: pinctrl.yaml# + +required: + - compatible + - reg + +additionalProperties: + anyOf: + - type: object + allOf: + - $ref: pincfg-node.yaml# + - $ref: pinmux-node.yaml# + + properties: + function: + enum: + [ + gpio, alt1, alt2, alt3, alt4, alt5, alt6, alt7, alt8, + aon_cpu_standbyb, aon_fp_4sec_resetb, aon_gpclk, aon_pwm, + arm_jtag, aud_fs_clk0, avs_pmu_bsc, bsc_m0, bsc_m1, bsc_m2, + bsc_m3, clk_observe, ctl_hdmi_5v, enet0, enet0_mii, enet0_rgmii, + ext_sc_clk, fl0, fl1, gpclk0, gpclk1, gpclk2, hdmi_tx0_auto_i2c, + hdmi_tx0_bsc, hdmi_tx1_auto_i2c, hdmi_tx1_bsc, i2s_in, i2s_out, + ir_in, mtsif, mtsif_alt, mtsif_alt1, pdm, pkt, pm_led_out, sc0, + sd0, sd2, sd_card_a, sd_card_b, sd_card_c, sd_card_d, sd_card_e, + sd_card_f, sd_card_g, spdif_out, spi_m, spi_s, sr_edm_sense, te0, + te1, tsio, uart0, uart1, uart2, usb_pwr, usb_vbus, uui, vc_i2c0, + vc_i2c3, vc_i2c4, vc_i2c5, vc_i2csl, vc_pcm, vc_pwm0, vc_pwm1, + vc_spi0, vc_spi3, vc_spi4, vc_spi5, vc_uart0, vc_uart2, vc_uart3, + vc_uart4, + ] + + pins: + items: + pattern: "^((aon_)?s?gpio[0-6]?[0-9])|(emmc_(clk|cmd|dat[0-7]|ds))$" + + bias-disable: true + bias-pull-down: true + bias-pull-up: true + additionalProperties: false + + - type: object + additionalProperties: + $ref: "#/additionalProperties/anyOf/0" +
It looks like they are few revisions of this chip which varies by number of pins. Perhaps not all of them are available on the market. Perhaps some of them where early engineering samples, I don't know. I decided to keep all of them just in case. Cc: Andrea della Porta <andrea.porta@suse.com> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de> --- .../pinctrl/brcm,brcmstb-pinctrl.yaml | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml