RE: [RFC 0/5] Renesas RZ series pinctrl driver

[Chris Brandt]

On Monday, January 30, 2017, Laurent Pinchart wrote:
> > It depends on the actual hardware: while per-pin settings are suitable
> > for SoCs that have per-pin hardware configuration (e.g. RZ/A1), it's
> > not suitable for SoCs where that's not the case, and where the
> > hardware has group-wise configuration (e.g. on R-Car).
> >
> > Hence IMHO "the future of Renesas pin control" will remain a split
> > personality for a while ;-)
> >
> > Laurent: please kick me if you disagree...
> 
> I unfortunately agree. Maybe we could try to speed up the process by
> providing feedback to the hardware engineers ?

For what it's worth, I fired off my 'rant and rave' last week about the
current R-Car PFC block so I'll be curious to see what they say. Of
course, I'm on the RZ side of the company, so maybe someone on the R-Car
side might have better luck.


Chris

Re: [RFC 0/5] Renesas RZ series pinctrl driver

[Laurent Pinchart]

Hi Geert,

On Monday 30 Jan 2017 17:08:11 Geert Uytterhoeven wrote:
> On Mon, Jan 30, 2017 at 2:51 PM, Linus Walleij wrote:
> > On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi wrote:
> >> after having discussed in great detail the RZ series per-pin PFC
> >> hardware peculiarities, this is a proposal for a possible pin-based pin
> >> controller driver for SoC devices of Renesas RZ family.
> >> 
> >> This RFC series adds a minimal driver infrastructure which supports pin
> >> multiplexing via explicit per-pin settings performed in device tree
> >> sources.
> >
> > I think this is what Laurent had in mind when he said something about
> > that he should have taken the per-pin approach from day 1.
> > 
> > I would especially like to see Laurent's review and ACK on this series
> > for that reason so we don't create something less than what he is
> > expecting for the future of Renesas pin control.
> 
> It depends on the actual hardware: while per-pin settings are suitable for
> SoCs that have per-pin hardware configuration (e.g. RZ/A1), it's not
> suitable for SoCs where that's not the case, and where the hardware has
> group-wise configuration (e.g. on R-Car).
> 
> Hence IMHO "the future of Renesas pin control" will remain a split
> personality for a while ;-)
> 
> Laurent: please kick me if you disagree...

I unfortunately agree. Maybe we could try to speed up the process by providing 
feedback to the hardware engineers ?

-- 
Regards,

Laurent Pinchart

Re: [RFC 0/5] Renesas RZ series pinctrl driver

[jacopo mondi]

Hi Tony,

On 30/01/2017 16:53, Tony Lindgren wrote:

* Linus Walleij  [170130 05:53]:

On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi  wrote:


   after having discussed in great detail the RZ series per-pin PFC hardware
peculiarities, this is a proposal for a possible pin-based pin controller
driver for SoC devices of Renesas RZ family.

This RFC series adds a minimal driver infrastructure which supports pin
multiplexing via explicit per-pin settings performed in device tree sources.


I think this is what Laurent had in mind when he said something about
that he should have taken the per-pin approach from day 1.

I would especially like to see Laurent's review and ACK on this series
for that reason so we don't create something less than what he is
expecting for the future of Renesas pin control.

The series as such seems fine and is reusing Tony's work to generalize
pin controllers putting functions and groups into the device tree in a nice
way, which makes it relevant for him to have a look as well, if he has
time.


Well I don't have the series in my inbox, but browsing through
the patchwork seems OK. The things to check would be:


Sorry about this, I thought sending to linux-gpio was enough.
We'll keep you in the loop now...



1. Do you guys really need RZ_PIN_DESC? Just let pinctrl
   framework generate the names, then do some simple user
   space tool that decodes the debugfs output. If you really
   need names, allow the consumer to set the name like we do
   for GPIOs and interrupts when requesting the resource.



Well, more than names I have used the RZ_PIN_DESC macro as an extended 
version of the PINCTRL_PIN one, to set bank and pin fields of the 
rz_pin_desc structure that describes a pin.


The proposed DTS syntax identifies pins by their position (bank:pin) 
instead that by index, so I need to have those information available in 
pin description to identify them.



2. Make sure you're using hardware offsets for any indexing
   to avoid adding artificial mapping tables between the driver
   and the the dts file. Many times new features are enabled
   between SoC revisions and a numbered index gets out of sync
   and needs constant patching just like we have for interrupt
   numbers earlier.. I don't think that's an issue with this
   series, but please check one more time for easy maintenance :)



As we describe pins "by position", if I got your comment properly, we 
should be safe, as indexes are purely incremental values from [0-npins].


I know pinctrl-single (and other drivers such as imx one if I'm not 
wrong) uses pin indexes to calculate register offsets. This series 
doesn't but uses the [bank:pin] couple for that purpose instead.


Thanks
   j



Regards,

Tony

Re: [RFC 0/5] Renesas RZ series pinctrl driver

[Tony Lindgren]

* Linus Walleij  [170130 05:53]:
> On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi  
> wrote:
> 
> >after having discussed in great detail the RZ series per-pin PFC hardware
> > peculiarities, this is a proposal for a possible pin-based pin controller
> > driver for SoC devices of Renesas RZ family.
> >
> > This RFC series adds a minimal driver infrastructure which supports pin
> > multiplexing via explicit per-pin settings performed in device tree sources.
> 
> I think this is what Laurent had in mind when he said something about
> that he should have taken the per-pin approach from day 1.
> 
> I would especially like to see Laurent's review and ACK on this series
> for that reason so we don't create something less than what he is
> expecting for the future of Renesas pin control.
> 
> The series as such seems fine and is reusing Tony's work to generalize
> pin controllers putting functions and groups into the device tree in a nice
> way, which makes it relevant for him to have a look as well, if he has
> time.

Well I don't have the series in my inbox, but browsing through
the patchwork seems OK. The things to check would be:

1. Do you guys really need RZ_PIN_DESC? Just let pinctrl
   framework generate the names, then do some simple user
   space tool that decodes the debugfs output. If you really
   need names, allow the consumer to set the name like we do
   for GPIOs and interrupts when requesting the resource.

2. Make sure you're using hardware offsets for any indexing
   to avoid adding artificial mapping tables between the driver
   and the the dts file. Many times new features are enabled
   between SoC revisions and a numbered index gets out of sync
   and needs constant patching just like we have for interrupt
   numbers earlier.. I don't think that's an issue with this
   series, but please check one more time for easy maintenance :)

Regards,

Tony

Re: [RFC 0/5] Renesas RZ series pinctrl driver

[Linus Walleij]

On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi  wrote:

>after having discussed in great detail the RZ series per-pin PFC hardware
> peculiarities, this is a proposal for a possible pin-based pin controller
> driver for SoC devices of Renesas RZ family.
>
> This RFC series adds a minimal driver infrastructure which supports pin
> multiplexing via explicit per-pin settings performed in device tree sources.

I think this is what Laurent had in mind when he said something about
that he should have taken the per-pin approach from day 1.

I would especially like to see Laurent's review and ACK on this series
for that reason so we don't create something less than what he is
expecting for the future of Renesas pin control.

The series as such seems fine and is reusing Tony's work to generalize
pin controllers putting functions and groups into the device tree in a nice
way, which makes it relevant for him to have a look as well, if he has
time.

Yours,
Linus Walleij

Re: [RFC 0/5] Renesas RZ series pinctrl driver

[Geert Uytterhoeven]

Hi Jacopo,

On Wed, Jan 25, 2017 at 7:09 PM, Jacopo Mondi  wrote:
>after having discussed in great detail the RZ series per-pin PFC hardware
> peculiarities, this is a proposal for a possible pin-based pin controller
> driver for SoC devices of Renesas RZ family.

"RZ" is a bad name, as there are (currently) 3 RZ subfamilies, and they
differ a lot. So it's better to use "RZ/A".
Or perhaps even "RZ/A1", as you never know what's going to happen with "2"...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

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

RE: [RFC 0/5] Renesas RZ series pinctrl driver

[Chris Brandt]

Hi Jacopo,

Thanks for the patches.

On Wednesday, January 25, 2017, Jacopo Mondi wrote:
> Right now, the only "SoC" module support implemented is for RZ/A1H (Genmai
> and GR-Peach boards).

I'm going to give it a try on the RZ/A1 RSK board.


> I have tested the correctness of mux settings printing out register values,
> and enabling/disabling the SCIF2 module connected to serial debug
> interface.

I can give it a try on things like Eth, I2C, SPI, SDHI, MMC.


> One note on the current DT ABI:
> right now a pin configuration is specified in DTS using utility macros
> defined in the (currently undocumented) arch/arm/boot/dts/include/dt-
> bindings/pinctrl/pinctrl-renesas-rz.h header file.
> Each pin configuration is a triplet of u32 in the form of
> 
> 
> 
> It should be fairly easy adding additional parameters to configure what
> was missing in the original group-based PFC driver for RZ devices (I'm
> thinking of IO mode control, input buffer configuration, bi-directional
> configuration etc).

SDHI is one that will need another parameter. Half the pins need Bidirection
(PBDC) enabled, and the other half need PBDC disabled.
SDHI data pins are PBDC=1, the others are PBDC=0.


> The series makes use of newly introduced pin[ctrl|mux]_generic functions,
> currently only available in Linus Walleij's linux-pinctrl.git tree.

Hmmm, I was hoping to 'easily' back port this to 4.9.
I guess if it's the best way forward, then maybe I can back port those new
generic functions for the LTSI tree.


Chris