Hi Vladimir,
(This time with the new email address ;-)
On Mon, Apr 27, 2020 at 9:41 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> Sorry for taking so long to get back to you, after our chat at Embedded
> Recipes.
>
> On Tue, Dec 18, 2018 at 1:58 PM Vladimir Zapolskiy
> <vladimir_zapolskiy@xxxxxxxxxx> wrote:
> > I'm still influenced by a use-case of competing access to a GPIO controller
> > from two OSes, there might be an overlapping with Linux PM routines in
> > the driver.
> >
> > As a side note I'm not convinced that gpiochip_line_is_valid() and
> > gpiochip->valid_mask usage in the driver is justified, unless it is agreed
> > that 'gpio-reserved-ranges' property is really supposed to describe "holes"
> > in GPIO controllers. The property found in r8a77470.dtsi (RZ/G1C) looks like
> > a kludge instead of making a proper assignment of 'gpio-ranges' property:
> >
> > - gpio-ranges = <&pfc 0 96 30>;
> > - gpio-reserved-ranges = <17 10>;
> > + gpio-ranges = <&pfc 0 96 17>, <&pfc 27 123 3>;
> >
> > The change above is untested and I have no access to RZ/G1C manual, it is
> > shared just to demonstrate an alternative idea of describing holes.
>
> Actually this is what Biju's v1 did[1]. But making that works means
> adding support for multiple "gpio-ranges" to the gpio-rcar driver. Of
> course that can be done, but it will complicate things, as all "offset"
> parameters in GPIO controller callbacks would need to take into account
> the holes when converting from contiguous offsets to discontiguous
> register bits.
>
> Note that the second tuple is <&pfc 27 123 3> not <&pfc 27 113 3>, i.e.
> the hole is not only present in the GPIO bank, but also in the pin
> controller's pin numbering (the PFC GPSR3 register has the same hole).
>
> Documentation/devicetree/bindings/gpio/gpio.txt says:
> | Some system-on-chips (SoCs) use the concept of GPIO banks. A GPIO bank is an
> | instance of a hardware IP core on a silicon die, usually exposed to the
> | programmer as a coherent range of I/O addresses. Usually each such bank is
> | exposed in the device tree as an individual gpio-controller node, reflecting
> | the fact that the hardware was synthesized by reusing the same IP block a
> | few times over.
>
> ... which applies to the R-Car GPIO block.
>
> | Optionally, a GPIO controller may have a "ngpios" property. This property
> | indicates the number of in-use slots of available slots for GPIOs. The
> | typical example is something like this: the hardware register is 32 bits
> | wide, but only 18 of the bits have a physical counterpart. The driver is
> | generally written so that all 32 bits can be used, but the IP block is reused
> | in a lot of designs, some using all 32 bits, some using 18 and some using
> | 12. In this case, setting "ngpios = <18>;" informs the driver that only the
> | first 18 GPIOs, at local offset 0 .. 17, are in use.
>
> So far the R-Car GPIO DT bindings never had a need for the "ngpios"
> property. The same information can easily be extracted from the last
> cell of the (single) mandatory "gpio-ranges" value.
>
> | If these GPIOs do not happen to be the first N GPIOs at offset 0...N-1, an
> | additional set of tuples is needed to specify which GPIOs are unusable, with
> | the gpio-reserved-ranges binding. This property indicates the start and size
> | of the GPIOs that can't be used.
>
> This also matches the case on RZ/G1C.
> So IMHO using "gpio-reserved-ranges" is appropriate for RZ/G1C.
>
> I do think the commit description of commit b9c725ed73b7cecc
> ("dt-bindings: gpio: Add a gpio-reserved-ranges property"):
>
> | Some qcom platforms make some GPIOs or pins unavailable for use
> | by non-secure operating systems, and thus reading or writing the
> | registers for those pins will cause access control issues.
> | Introduce a DT property to describe the set of GPIOs that are
> | available for use so that higher level OSes are able to know what
> | pins to avoid reading/writing.
>
> is confusing, as it only talks about pins that can be used by a secure
> OS only, but the actual changes to gpio.txt are in-line with GPIO lines
> that are simply not wired to physical pins, which is what the original
> paragraph was talking about, too.
>
> What do other people think?
> Thanks!
>
> [1] [PATCH 2/4] ARM: dts: r8a77470: Add GPIO support
> https://lore.kernel.org/linux-devicetree/1532685089-35645-3-git-send-email-biju.das@xxxxxxxxxxxxxx/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
