From: "Álvaro Fernández Rojas" <noltari@gmail.com> To: Linus Walleij <linus.walleij@linaro.org>, Rob Herring <robh+dt@kernel.org> Cc: Michael Walle <michael@walle.cc>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Florian Fainelli <f.fainelli@gmail.com>, "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" <bcm-kernel-feedback-list@broadcom.com>, Jonas Gorski <jonas.gorski@gmail.com>, Necip Fazil Yildiran <fazilyildiran@gmail.com>, Andy Shevchenko <andy.shevchenko@gmail.com>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v6 03/15] pinctrl: bcm: add bcm63xx base code Date: Thu, 11 Mar 2021 18:00:40 +0100 [thread overview] Message-ID: <edb31609-e138-9844-7168-004c882cae97@gmail.com> (raw) In-Reply-To: <CACRpkdaMYy_Z34i+0uRgciC=xBtoeNsWViHU9ZysvxqrFXB5+w@mail.gmail.com> Hi Rob and Linus, El 11/03/2021 a las 17:13, Linus Walleij escribió: > On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote: >> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote: >>> >>>>> +static const struct of_device_id bcm63xx_gpio_of_match[] = { >>>>> + { .compatible = "brcm,bcm6318-gpio", }, >>>>> + { .compatible = "brcm,bcm6328-gpio", }, >>>>> + { .compatible = "brcm,bcm6358-gpio", }, >>>>> + { .compatible = "brcm,bcm6362-gpio", }, >>>>> + { .compatible = "brcm,bcm6368-gpio", }, >>>>> + { .compatible = "brcm,bcm63268-gpio", }, >>>> >>>> All these would be moved to gpio-mmio.c (or maybe that can have a >>>> fallback compatible?). >>> >>> This is gpio-regmap.c and it can only be used as a library >>> by a certain driver. gpio-mmio.c can be used stand-alone >>> for certain really simple hardware (though most use that >>> as a library as well). >> >> I don't really care which one is used, but the problem is that this >> choice is leaking into the binding design. > > Aha I guess I misunderstood your comment. > >> The primary problem here is >> once someone uses regmap, then they think they must have a syscon and >> can abandon using 'reg' and normal address properties as Linux happens >> to not use them (currently). I think we really need some better regmap >> vs. mmio handling to eliminate this duplication of foo-mmio and >> foo-regmap drivers and difference in binding design. Not sure exactly >> what that looks like, but basically some sort of 'reg' property to >> regmap creation. > > I see the problem. Yeah we should try to be more strict around > these things. To me there are syscons and "other regmaps", > where syscon is a real hurdle of registers while "other regmaps" > are just regmaps by convenience. > > Documentation/devicetree/bindings/mfd/syscon.yaml > describes what a syscon really is so if everyone could > just read the documentation that would be great ... > >> Given we already have a Broadcom GPIO binding for what looks to be >> similar to this one, I'm left wondering what's the real difference >> here? > > Which one is similar? I can take a look. @Linus I think @Rob is referring to brcm,bcm6345-gpio: https://github.com/torvalds/linux/blob/a74e6a014c9d4d4161061f770c9b4f98372ac778/drivers/gpio/gpio-mmio.c#L686 However, the real difference between BCM6345 (and BCM6338) is that these SoCs have no pin controller at all, only a GPIO controller: BCM6345: typedef struct GpioControl { uint16 unused0; byte unused1; byte TBusSel; uint16 unused2; uint16 GPIODir; byte unused3; byte Leds; uint16 GPIOio; uint32 UartCtl; } GpioControl; BCM6338: typedef struct GpioControl { uint32 unused0; uint32 GPIODir; /* bits 7:0 */ uint32 unused1; uint32 GPIOio; /* bits 7:0 */ uint32 LEDCtrl; uint32 SpiSlaveCfg; uint32 vRegConfig; } GpioControl; BCM6348 and newer also have pinctrl. That's the main difference between that driver @Rob's referring to and the ones in this patch series. > > We currently have four Broadcom GPIO bindings, > which are stand alone GPIO blocks and eight Broadcom > pin controllers that all do GPIO as well. > > This family of pin controllers are (as per subject) is > the bcm63xx series which is a MIPS-based family of SoCs > found in routers, top bindings in > Documentation/devicetree/bindings/mips/brcm/soc.txt > These all have a GPIO block as part of the pin controller > and the GPIO block is a distinct sub-function of the > pin controller, and it has up to 32 GPIOs per block, > hence it has its own subnode inside the pin controller. > > This driver follows the pattern of the Ingenic > pin controller, another MIPS SoC: > Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml > > Another SoC with several GPIO blocks inside the pin > controller is SparX5 and that also follows this pattern: > Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml > (This has an example with more than one GPIO block > inside the pin controller.) > > Yours, > Linus Walleij >
WARNING: multiple messages have this Message-ID (diff)
From: "Álvaro Fernández Rojas" <noltari@gmail.com> To: Linus Walleij <linus.walleij@linaro.org>, Rob Herring <robh+dt@kernel.org> Cc: Michael Walle <michael@walle.cc>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Florian Fainelli <f.fainelli@gmail.com>, "maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" <bcm-kernel-feedback-list@broadcom.com>, Jonas Gorski <jonas.gorski@gmail.com>, Necip Fazil Yildiran <fazilyildiran@gmail.com>, Andy Shevchenko <andy.shevchenko@gmail.com>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, linux-arm-kernel <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v6 03/15] pinctrl: bcm: add bcm63xx base code Date: Thu, 11 Mar 2021 18:00:40 +0100 [thread overview] Message-ID: <edb31609-e138-9844-7168-004c882cae97@gmail.com> (raw) In-Reply-To: <CACRpkdaMYy_Z34i+0uRgciC=xBtoeNsWViHU9ZysvxqrFXB5+w@mail.gmail.com> Hi Rob and Linus, El 11/03/2021 a las 17:13, Linus Walleij escribió: > On Thu, Mar 11, 2021 at 3:58 PM Rob Herring <robh+dt@kernel.org> wrote: >> On Wed, Mar 10, 2021 at 6:09 PM Linus Walleij <linus.walleij@linaro.org> wrote: >>> On Wed, Mar 10, 2021 at 6:51 PM Rob Herring <robh+dt@kernel.org> wrote: >>> >>>>> +static const struct of_device_id bcm63xx_gpio_of_match[] = { >>>>> + { .compatible = "brcm,bcm6318-gpio", }, >>>>> + { .compatible = "brcm,bcm6328-gpio", }, >>>>> + { .compatible = "brcm,bcm6358-gpio", }, >>>>> + { .compatible = "brcm,bcm6362-gpio", }, >>>>> + { .compatible = "brcm,bcm6368-gpio", }, >>>>> + { .compatible = "brcm,bcm63268-gpio", }, >>>> >>>> All these would be moved to gpio-mmio.c (or maybe that can have a >>>> fallback compatible?). >>> >>> This is gpio-regmap.c and it can only be used as a library >>> by a certain driver. gpio-mmio.c can be used stand-alone >>> for certain really simple hardware (though most use that >>> as a library as well). >> >> I don't really care which one is used, but the problem is that this >> choice is leaking into the binding design. > > Aha I guess I misunderstood your comment. > >> The primary problem here is >> once someone uses regmap, then they think they must have a syscon and >> can abandon using 'reg' and normal address properties as Linux happens >> to not use them (currently). I think we really need some better regmap >> vs. mmio handling to eliminate this duplication of foo-mmio and >> foo-regmap drivers and difference in binding design. Not sure exactly >> what that looks like, but basically some sort of 'reg' property to >> regmap creation. > > I see the problem. Yeah we should try to be more strict around > these things. To me there are syscons and "other regmaps", > where syscon is a real hurdle of registers while "other regmaps" > are just regmaps by convenience. > > Documentation/devicetree/bindings/mfd/syscon.yaml > describes what a syscon really is so if everyone could > just read the documentation that would be great ... > >> Given we already have a Broadcom GPIO binding for what looks to be >> similar to this one, I'm left wondering what's the real difference >> here? > > Which one is similar? I can take a look. @Linus I think @Rob is referring to brcm,bcm6345-gpio: https://github.com/torvalds/linux/blob/a74e6a014c9d4d4161061f770c9b4f98372ac778/drivers/gpio/gpio-mmio.c#L686 However, the real difference between BCM6345 (and BCM6338) is that these SoCs have no pin controller at all, only a GPIO controller: BCM6345: typedef struct GpioControl { uint16 unused0; byte unused1; byte TBusSel; uint16 unused2; uint16 GPIODir; byte unused3; byte Leds; uint16 GPIOio; uint32 UartCtl; } GpioControl; BCM6338: typedef struct GpioControl { uint32 unused0; uint32 GPIODir; /* bits 7:0 */ uint32 unused1; uint32 GPIOio; /* bits 7:0 */ uint32 LEDCtrl; uint32 SpiSlaveCfg; uint32 vRegConfig; } GpioControl; BCM6348 and newer also have pinctrl. That's the main difference between that driver @Rob's referring to and the ones in this patch series. > > We currently have four Broadcom GPIO bindings, > which are stand alone GPIO blocks and eight Broadcom > pin controllers that all do GPIO as well. > > This family of pin controllers are (as per subject) is > the bcm63xx series which is a MIPS-based family of SoCs > found in routers, top bindings in > Documentation/devicetree/bindings/mips/brcm/soc.txt > These all have a GPIO block as part of the pin controller > and the GPIO block is a distinct sub-function of the > pin controller, and it has up to 32 GPIOs per block, > hence it has its own subnode inside the pin controller. > > This driver follows the pattern of the Ingenic > pin controller, another MIPS SoC: > Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.yaml > > Another SoC with several GPIO blocks inside the pin > controller is SparX5 and that also follows this pattern: > Documentation/devicetree/bindings/pinctrl/microchip,sparx5-sgpio.yaml > (This has an example with more than one GPIO block > inside the pin controller.) > > Yours, > Linus Walleij > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-11 17:01 UTC|newest] Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-10 12:54 [PATCH v6 00/15] pinctrl: add BCM63XX pincontrol support Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 01/15] gpio: guard gpiochip_irqchip_add_domain() with GPIOLIB_IRQCHIP Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 02/15] gpio: regmap: set gpio_chip of_node Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 14:01 ` Andy Shevchenko 2021-03-10 14:01 ` Andy Shevchenko 2021-03-10 18:27 ` Michael Walle 2021-03-10 18:27 ` Michael Walle 2021-03-10 19:12 ` Álvaro Fernández Rojas 2021-03-10 19:12 ` Álvaro Fernández Rojas 2021-03-11 1:16 ` Linus Walleij 2021-03-11 1:16 ` Linus Walleij 2021-03-10 12:54 ` [PATCH v6 03/15] pinctrl: bcm: add bcm63xx base code Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 14:07 ` Andy Shevchenko 2021-03-10 14:07 ` Andy Shevchenko 2021-03-10 17:50 ` Rob Herring 2021-03-10 17:50 ` Rob Herring 2021-03-11 1:09 ` Linus Walleij 2021-03-11 1:09 ` Linus Walleij 2021-03-11 14:57 ` Rob Herring 2021-03-11 14:57 ` Rob Herring 2021-03-11 16:13 ` Linus Walleij 2021-03-11 16:13 ` Linus Walleij 2021-03-11 17:00 ` Álvaro Fernández Rojas [this message] 2021-03-11 17:00 ` Álvaro Fernández Rojas 2021-03-11 18:24 ` Rob Herring 2021-03-11 18:24 ` Rob Herring 2021-03-11 18:32 ` Álvaro Fernández Rojas 2021-03-11 18:32 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 04/15] dt-bindings: add BCM6328 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 17:45 ` Rob Herring 2021-03-10 17:45 ` Rob Herring 2021-03-10 18:03 ` Álvaro Fernández Rojas 2021-03-10 18:03 ` Álvaro Fernández Rojas 2021-03-10 18:45 ` Rob Herring 2021-03-10 18:45 ` Rob Herring 2021-03-10 19:10 ` Álvaro Fernández Rojas 2021-03-10 19:10 ` Álvaro Fernández Rojas 2021-03-10 20:52 ` Rob Herring 2021-03-10 20:52 ` Rob Herring 2021-03-11 17:09 ` Álvaro Fernández Rojas 2021-03-11 17:09 ` Álvaro Fernández Rojas 2021-03-11 18:14 ` Rob Herring 2021-03-11 18:14 ` Rob Herring 2021-03-11 18:34 ` Álvaro Fernández Rojas 2021-03-11 18:34 ` Álvaro Fernández Rojas 2021-03-15 15:57 ` Linus Walleij 2021-03-15 15:57 ` Linus Walleij 2021-03-11 1:22 ` Linus Walleij 2021-03-11 1:22 ` Linus Walleij 2021-03-10 12:54 ` [PATCH v6 05/15] pinctrl: add a pincontrol driver for BCM6328 Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 06/15] dt-bindings: add BCM6358 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 07/15] pinctrl: add a pincontrol driver for BCM6358 Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 08/15] dt-bindings: add BCM6362 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 09/15] pinctrl: add a pincontrol driver for BCM6362 Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 10/15] dt-bindings: add BCM6368 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:54 ` [PATCH v6 11/15] pinctrl: add a pincontrol driver for BCM6368 Álvaro Fernández Rojas 2021-03-10 12:54 ` Álvaro Fernández Rojas 2021-03-10 12:55 ` [PATCH v6 12/15] dt-bindings: add BCM63268 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:55 ` Álvaro Fernández Rojas 2021-03-10 12:55 ` [PATCH v6 13/15] pinctrl: add a pincontrol driver for BCM63268 Álvaro Fernández Rojas 2021-03-10 12:55 ` Álvaro Fernández Rojas 2021-03-10 12:55 ` [PATCH v6 14/15] dt-bindings: add BCM6318 pincontroller binding documentation Álvaro Fernández Rojas 2021-03-10 12:55 ` Álvaro Fernández Rojas 2021-03-10 12:55 ` [PATCH v6 15/15] pinctrl: add a pincontrol driver for BCM6318 Álvaro Fernández Rojas 2021-03-10 12:55 ` Álvaro Fernández Rojas
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=edb31609-e138-9844-7168-004c882cae97@gmail.com \ --to=noltari@gmail.com \ --cc=andy.shevchenko@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bgolaszewski@baylibre.com \ --cc=devicetree@vger.kernel.org \ --cc=f.fainelli@gmail.com \ --cc=fazilyildiran@gmail.com \ --cc=jonas.gorski@gmail.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=michael@walle.cc \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.