Re: [PATCH] pinctrl: Add one-register-per-pin type device tree based pinctrl driver

[Stephen Warren <swarren@wwwdotorg.org>]

On 06/22/2012 02:39 AM, Tony Lindgren wrote:
> Hi,
> 
> * Stephen Warren <swarren@wwwdotorg.org> [120621 15:17]:
>> On 06/19/2012 07:56 AM, Tony Lindgren wrote:
>>> +
>>> +- pinctrl-single,pinconf-mask : mask of allowed pinconf bits in the
>>> +  pinmux register; this gets combined with pinconf mask but is a separate
>>> +  mask to allow the option of setting pinconf separatately from the
>>> +  function
>>
>> Given that this binding doesn't allow describing pin configuration at
>> present, I would simply remove all mention of that property in the
>> binding documentation. It can be added back if/when that feature is
>> added. Any future driver using this binding can refuse to allow pin
>> configuration if that property is missing.
> 
> It might be better to just add support for pin_config_get/set to avoid
> changing the binding later:
> 
>  static int pcs_pinconf_get(struct pinctrl_dev *pctldev,
>                                 unsigned pin, unsigned long *config)
>  {
> -	return -ENOTSUPP;
> +	struct pcs_device *pcs;
> +	void __iomem *reg;
> +	int res;
> +
> +	pcs = pinctrl_dev_get_drvdata(pctldev);
> +	res = pcs_pin_to_reg(pcs, pin, &reg);
> +	if (res)
> +		return res;
> +
> +	return pcs->read(reg) & pcs->cmask;
>  }
> 
> A have not done that yet as currently the pcs_pin_to_reg() would need to be
> sorted out in somewhat clean manner.

Yes, implementing pinconf in the driver and binding would be a fine
alternative; I just assumed you'd want to defer that. How would pinconf
be represented in DT; just extra bits set in the value portion of the
pins property? Thinking quickly, I guess that would work fine, since the
binding's assumption is presumably that there's a 1:1 mapping between
the set of pins that functions can be mux'd onto and the set of pins
that can have pinconf applied, and the register to do both muxing and
pinconf is the same.