[PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
Anthony Bagwell
anthony.bagwell at hivehome.com
Mon Jan 17 09:52:03 CET 2022
Thank you for the review. Sorry I forgot to add the sign off. Here is my signoff line.
Signed-off-by: Anthony Bagwell <anthony.bagwell at hivehome.com>
On 14/01/2022, 18:17, "Tom Rini" <trini at konsulko.com> wrote:
On Fri, Dec 03, 2021 at 03:18:53PM +0000, AJ Bagwell wrote:
> Changes to the am33xx device (33e9021a) trees have been merged in from
> the upstream linux kernel which now means the device tree uses the new
> pins format (as of 5.10) where the confinguration can be stores as a
> separate configuration value and pin mux mode which are then OR'd
> together.
>
> This patch adds support for the new format to u-boot so that
> pinctrl-cells is now respected when reading in pinctrl-single,pins
> ---
> drivers/pinctrl/pinctrl-single.c | 55 +++++++++++++++++---------------
> 1 file changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 8fc07e3498..bc9c17bce8 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -28,6 +28,7 @@ struct single_pdata {
> int offset;
> u32 mask;
> u32 width;
> + u32 args_count;
> bool bits_per_mux;
> };
>
> @@ -78,20 +79,6 @@ struct single_priv {
> struct list_head gpiofuncs;
> };
>
> -/**
> - * struct single_fdt_pin_cfg - pin configuration
> - *
> - * This structure is used for the pin configuration parameters in case
> - * the register controls only one pin.
> - *
> - * @reg: configuration register offset
> - * @val: configuration register value
> - */
> -struct single_fdt_pin_cfg {
> - fdt32_t reg;
> - fdt32_t val;
> -};
> -
> /**
> * struct single_fdt_bits_cfg - pin configuration
> *
> @@ -314,25 +301,28 @@ static int single_pin_compare(const void *s1, const void *s2)
> * @dev: Pointer to single pin configuration device which is the parent of
> * the pins node holding the pin configuration data.
> * @pins: Pointer to the first element of an array of register/value pairs
> - * of type 'struct single_fdt_pin_cfg'. Each such pair describes the
> - * the pin to be configured and the value to be used for configuration.
> + * of type 'u32'. Each such pair describes the pin to be configured
> + * and the value to be used for configuration.
> + * The value can either be a simple value if #pinctrl-cells = 1
> + * or a configuration value and a pin mux mode value if it is 2
> * This pointer points to a 'pinctrl-single,pins' property in the
> * device-tree.
> * @size: Size of the 'pins' array in bytes.
> - * The number of register/value pairs in the 'pins' array therefore
> - * equals to 'size / sizeof(struct single_fdt_pin_cfg)'.
> + * The number of cells in the array therefore equals to
> + * 'size / sizeof(u32)'.
> * @fname: Function name.
> */
> static int single_configure_pins(struct udevice *dev,
> - const struct single_fdt_pin_cfg *pins,
> + const u32 *pins,
> int size, const char *fname)
> {
> struct single_pdata *pdata = dev_get_plat(dev);
> struct single_priv *priv = dev_get_priv(dev);
> - int n, pin, count = size / sizeof(struct single_fdt_pin_cfg);
> + int stride = pdata->args_count + 1;
> + int n, pin, count = size / sizeof(u32);
> struct single_func *func;
> phys_addr_t reg;
> - u32 offset, val;
> + u32 offset, val, mux;
>
> /* If function mask is null, needn't enable it. */
> if (!pdata->mask)
> @@ -344,16 +334,22 @@ static int single_configure_pins(struct udevice *dev,
>
> func->name = fname;
> func->npins = 0;
> - for (n = 0; n < count; n++, pins++) {
> - offset = fdt32_to_cpu(pins->reg);
> + for (n = 0; n < count; n += stride) {
> + offset = fdt32_to_cpu(pins[n]);
> if (offset > pdata->offset) {
> dev_err(dev, " invalid register offset 0x%x\n",
> offset);
> continue;
> }
>
> + /* if the pinctrl-cells is 2 then the second cell contains the mux */
> + if (stride == 3)
> + mux = fdt32_to_cpu(pins[n + 2]);
> + else
> + mux = 0;
> +
> reg = pdata->base + offset;
> - val = fdt32_to_cpu(pins->val) & pdata->mask;
> + val = (fdt32_to_cpu(pins[n + 1]) | mux) & pdata->mask;
> pin = single_get_pin_by_offset(dev, offset);
> if (pin < 0) {
> dev_err(dev, " failed to get pin by offset %x\n",
> @@ -453,7 +449,7 @@ static int single_configure_bits(struct udevice *dev,
> static int single_set_state(struct udevice *dev,
> struct udevice *config)
> {
> - const struct single_fdt_pin_cfg *prop;
> + const u32 *prop;
> const struct single_fdt_bits_cfg *prop_bits;
> int len;
>
> @@ -461,7 +457,7 @@ static int single_set_state(struct udevice *dev,
>
> if (prop) {
> dev_dbg(dev, "configuring pins for %s\n", config->name);
> - if (len % sizeof(struct single_fdt_pin_cfg)) {
> + if (len % sizeof(u32)) {
> dev_dbg(dev, " invalid pin configuration in fdt\n");
> return -FDT_ERR_BADSTRUCTURE;
> }
> @@ -612,6 +608,13 @@ static int single_of_to_plat(struct udevice *dev)
>
> pdata->bits_per_mux = dev_read_bool(dev, "pinctrl-single,bit-per-mux");
>
> + /* If no pinctrl-cells is present, default to old style of 2 cells with
> + * bits per mux and 1 cell otherwise.
> + */
> + ret = dev_read_u32(dev, "#pinctrl-cells", &pdata->args_count);
> + if (ret)
> + pdata->args_count = pdata->bits_per_mux ? 2 : 1;
> +
> return 0;
> }
While this change seems fine, it is missing a Signed-off-by line.
Please see https://developercertificate.org/ and if so, either repost or
reply with your line here as the change otherwise still applies fine,
thanks.
--
Tom
The information contained in or attached to this email is intended only for the use of the individual or entity to which it is addressed. If you are not the intended recipient, or a person responsible for delivering it to the intended recipient, you are not authorised to and must not disclose, copy, distribute, or retain this message or any part of it. It may contain information which is confidential and/or covered by legal professional or other privilege under applicable law.
The views expressed in this email are not necessarily the views of Centrica plc or its subsidiaries and those companies, their directors, officers or employees make no representation, nor accept any liability, regarding its accuracy or completeness, unless expressly stated to the contrary.
Additional regulatory disclosures may be found here: https://www.centrica.com/site-tools/privacy-cookies-and-legal-disclaimer/
PH Jones is a trading name of British Gas Social Housing Limited. British Gas Social Housing Limited (company no: 01026007), British Gas Trading Limited (company no: 03078711), British Gas Services Limited (company no: 3141243), British Gas Insurance Limited (company no: 06608316), British Gas New Heating Limited (company no: 06723244), British Gas Services (Commercial) Limited (company no: 07385984) and Centrica Energy (Trading) Limited (company no: 02877397) are all wholly owned subsidiaries of Centrica plc (company no: 3033654). Each company is registered in England and Wales with a registered office at Millstream, Maidenhead Road, Windsor, Berkshire SL4 5GD.
British Gas Insurance Limited is authorised by the Prudential Regulation Authority and regulated by the Financial Conduct Authority and the Prudential Regulation Authority. British Gas Services Limited is authorised and regulated by the Financial Conduct Authority. British Gas Trading Limited is an appointed representative of British Gas Services Limited which is authorised and regulated by the Financial Conduct Authority.
Bord Gáis Energy Limited is a wholly owned subsidiary of Centrica Plc (company number 463078), registered in Ireland with registered office at One Warrington Place, Dublin 2.
You can find a list of our privacy notices here: https://www.centrica.com/site-tools/privacy-cookies-and-legal-disclaimer/
More information about the U-Boot
mailing list