On Tue, Jun 16, 2020 at 07:26:28AM -0700, Tony Lindgren wrote:
> * Drew Fustini <drew@xxxxxxxxxxxxxxx> [200615 23:10]:
> > These changes are based on feedback from Tony [1] concerning changing
> > pinctrl-single to be able to handle pinctrl-cells = 2 for for the
> > "pinctrl-single,pins" property.
>
> Hey this is great! Thanks for sorting it out.
>
> > --- a/drivers/pinctrl/pinctrl-single.c
> > +++ b/drivers/pinctrl/pinctrl-single.c
> > @@ -1017,11 +1017,21 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
> > break;
> > }
> >
> > - /* Index plus one value cell */
> > offset = pinctrl_spec.args[0];
> > vals[found].reg = pcs->base + offset;
> > - vals[found].val = pinctrl_spec.args[1];
> >
> > + pr_info("%s: args_count=%d offset=0x%x", __func__,
> > + pinctrl_spec.args_count, offset);
> > + pr_info("%s: args[1]=0x%x", __func__, pinctrl_spec.args[1]);
> > +
> > + if (pinctrl_spec.args_count == 2) {
> > + vals[found].val = pinctrl_spec.args[1];
> > + } else if (pinctrl_spec.args_count == 3) {
> > + pr_info("%s: args[2]=0x%x", __func__, pinctrl_spec.args[2]);
> > + vals[found].val = (pinctrl_spec.args[1] | pinctrl_spec.args[2]);
> > + }
>
> Maybe do the above with a switch? And leave out the pr_info for the
> final version :) Also please do a separate patch for pinctrl-singl.c,
> and then another patch for the define and dts change.
Thanks for the suggestions. Yes, I wasn't sure what the best way to
express this "rough draft" solution. I'll create a patch series in
future.
> Hmm so now the conf and mux values are still register masks in the dts
> which is not ideal in all cases. But that's a separate issue and could
> be sorted out as needed later on with adding separate conf and mux
> shifts and masks. Not sure if we want to do that for the existing use
> cases though.
>
> Regards,
>
> Tony
By register masks, do you mean the #define's like PIN_OUTPUT_PULLDOWN
and MUX_MODE6?
AM33XX_PADCONF(AM335X_PIN_GPMC_A2, PIN_OUTPUT_PULLDOWN, MUX_MODE6)
There is a possibility to also use the pinconf properties defined in
pinctrl-single.c:
static const struct pcs_conf_type prop2[] = {
{ "pinctrl-single,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, },
{ "pinctrl-single,slew-rate", PIN_CONFIG_SLEW_RATE, },
{ "pinctrl-single,input-schmitt", PIN_CONFIG_INPUT_SCHMITT, },
{ "pinctrl-single,low-power-mode", PIN_CONFIG_LOW_POWER_MODE, },
};
static const struct pcs_conf_type prop4[] = {
{ "pinctrl-single,bias-pullup", PIN_CONFIG_BIAS_PULL_UP, },
{ "pinctrl-single,bias-pulldown", PIN_CONFIG_BIAS_PULL_DOWN, },
{ "pinctrl-single,input-schmitt-enable",
PIN_CONFIG_INPUT_SCHMITT_ENABLE, },
};
For example, I was experimenting with how to define the bias properties:
ehrpwm0_pins: pinmux-ehrpwm0-pins {
pinctrl-single,pins = <
AM33XX_PADCONF(AM335X_PIN_MCASP0_ACLKX, PIN_OUTPUT_PULLDOWN, MUX_MODE1)
/* (A13) mcasp0_aclkx.ehrpwm0A */
>;
pinctrl-single,bias-pullup = <24 24 0 24>;
};
ehrpwm1_pins: pinmux-ehrpwm1-pins {
pinctrl-single,pins = <
AM33XX_PADCONF(AM335X_PIN_GPMC_A2, PIN_OUTPUT_PULLDOWN, MUX_MODE6)
/* (U14) gpmc_a2.ehrpwm1A */
>;
pinctrl-single,bias-pulldown = <8 8 0 24>;
};
I found the binding documentation [0] for the bias properties to be very
confusing as to how those 4 values work:
> - pinctrl-single,bias-pullup : array of value that are used to configure the
> input bias pullup in the pinmux register.
>
> /* input, enabled pullup bits, disabled pullup bits, mask */
> pinctrl-single,bias-pullup = <0 1 0 1>;
>
> - pinctrl-single,bias-pulldown : array of value that are used to configure the
> input bias pulldown in the pinmux register.
> /* input, enabled pulldown bits, disabled pulldown bits, mask */
> pinctrl-single,bias-pulldown = <2 2 0 2>;
For AM3358, the pin conf
register has the format [1]:
bit attribute value
6 slew { 0: fast, 1: slow }
5 rx_active { 0: rx disable, 1: rx enabled }
4 pu_typesel { 0: pulldown select, 1: pullup select }
3 puden { 0: pud enable, 1: disabled }
2 mode 3 bits to selec mode 0 to 7
1 mode
0 mode
And I figured out the values for the bias-pull{up,down} properties:
16 8 4 2 1
2^4 2^3 2^2 2^1 2^0
mask 1 1 0 0 0 24
pull-up 1 1 0 0 0 24
pull-dn 0 1 0 0 0 8
none 0 0 0 0 0 0
I did some testing with pr_info's sprinkled in and I think those values
are correct but I would be happy to hear from someone with more insight
in the design of bias-pulldown and bias-pullup.
thanks,
drew
[0] Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
[1] https://www.ti.com/lit/ug/spruh73q/spruh73q.pdf (see Figure 9-51)
