Neil Armstrong <narmstrong@xxxxxxxxxxxx> writes: > On 03/14/2017 04:42 PM, Linus Walleij wrote: >> On Thu, Mar 9, 2017 at 8:47 PM, Martin Blumenstingl >> <martin.blumenstingl@xxxxxxxxxxxxxx> wrote: >>> On Mon, Mar 6, 2017 at 3:42 PM, Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote: >>>> On Sat, 2017-03-04 at 22:23 +0100, Martin Blumenstingl wrote: >> >>>>> + FUNCTION(pwm_f_clk), >>>>> + FUNCTION(pwm_f_x), >>>> >>>> I wonder if having function named "pwm_f_clk" really makes sense ? >>>> Shouldn't it be just "pwm_f" ? This is real function, isn't it ? >>>> The actual pin used will be provided in the dt. Here, I suppose we >>>> could have this: >>>> >>>> +static const char * const pwm_f_groups[] = { >>>> + "pwm_f_x", "pwm_f_clk", >>>> +}; >>>> >>>> Has far as I can see, on meson arch, the function does not carry much >>>> information anyway, except for prints. >>>> >>>> To be clear, I'm not questioning this change in particular. It looks >>>> good, and follows what has been done in the past on meson. I know we >>>> have been this a lot already, but I'm questioning whether we should >>>> continue to do so ? >>>> >>>> I asking because I also have a lot case like this coming up on audio >>>> for gxl and gxbb, where the same function can use different pins. >>> >>> could you please look into Jerome's question? >>> personally I'm fine with either way, and changing my patch would be >>> quite trivial. but I'd like to know what's "the way to go" before >>> changing anything (and reverting that afterwards again). >> >> I don't understand the question really. >> >> I am not an expert on this system, if the people working with it >> cannot tell a function from a group I don't know who can... certainly >> not me. >> >> What I can say is that pincontrol combines functions and groups to >> states using a mapping. The functions should be something you poke >> into a register, the groups are looser defined but may also be a >> character of the hardware, but more usual a character of the >> intended electronic usecase. Groups contain 1..n pins and can >> be combined with some applicable functions. >> >> Please re-read Documentation/pinctrl.txt very closely if anything is >> unclear, I really put a lot of hours into getting that right. Especially >> reexamine "Pinmux conventions". > > The point pushed by Jerome was purely cosmetic since the groups in the meson > pinctrl driver are purely cosmetic, since only the GPIO group is handled, > other groups are all handled the same. handled the same... as what? > This is because I pushed all the PWM pins in a separate group, but functionnaly > the internal signal (i.e. PWM F) is the same for multiple pins and should be > a single "PWM F" group instead of multiple ones. > > My advice is to leave the PWM groups as is, Do you mean as we have in mainline today? or as is proposed in $SUBJECT patch? > and push new pins/functions/groups > grouped with the internal signal name if split on multiple pins. Can somone do a quick patch for PWM_F for example, also showing how this will look in the DT if someone wants to switch between the PWM_F on GPIOX or GPIOCLK? We shouldalso verify that the driver is detecting/removing conflicts properly when something else is already using that pin (e.g. SDIO_IRQ shares pin GPIOX_7 with PWM_F) Kevin -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html