Re: [PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
On Fri, Dec 03, 2021 at 03:18:53PM +, 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 > Signed-off-by: Anthony Bagwell Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
Thank you for the review. Sorry I forgot to add the sign off. Here is my
signoff line.
Signed-off-by: Anthony Bagwell
On 14/01/2022, 18:17, "Tom Rini" wrote:
On Fri, Dec 03, 2021 at 03:18:53PM +, 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
Re: [PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
On Fri, Dec 03, 2021 at 03:18:53PM +, 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,
[PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
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
[PATCH] pinctrl: single: add support for pinctrl-single, pins when #pinctrl-cells = 2
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
