* Drew Fustini <drew@xxxxxxxxxxxxxxx> [200608 20:23]: > I've been looking at pinctrl-single.c further and now I am wondering if > it makes sense to modify pcs_parse_one_pinctrl_entry() or create a new > function. Currently it does: > > 1020 /* Index plus one value cell */ > 1021 offset = pinctrl_spec.args[0]; > 1022 vals[found].reg = pcs->base + offset; > 1023 vals[found].val = pinctrl_spec.args[1]; > > I believe it will need something similar to what is done in > pcs_parse_bits_in_pinctrl_entry(): > > 1136 /* Index plus two value cells */ > 1137 offset = pinctrl_spec.args[0]; > 1138 val = pinctrl_spec.args[1]; > 1139 mask = pinctrl_spec.args[2]; > > So using #pinctrl-cells = <2> would require would require: > > offset = pinctrl_spec.args[0]; > vals[found].reg = pcs->base + offset; > vals[found].conf = pinctrl_spec.args[1]; > vals[found].mux = pinctrl_spec.args[2]; > > However, the type of vals: > > struct pcs_func_vals { > void __iomem *reg; > unsigned val; > unsigned mask; > }; > > represents the combined mux + conf value in the register, so I think > pcs_conf_vals would need to used: > > struct pcs_conf_vals { > enum pin_config_param param; > unsigned val; > unsigned enable; > unsigned disable; > unsigned mask; > }; > > Thoughts? Yes sounds about right to me. On write the driver would just write the register based on mux and conf and orr the values together using configured shifts. Not sure if #pinctrl-cells would ever be more than 2 in this case, but having things generic where possible makes sense. For example, the omap padconf wakeirq specific bits could potentially configured with #pinctrl-cells = <3>. We now handle them automatically as interrupts so there's really nothing to configure there. I don't if other SoCs might need to configure other options beyond conf and mux in the dts. I guess it's safe to assume nothing else needs to be configured and have the folks needing more support add it as needed. Regards, Tony