[PATCH v1 0/3] extend pinctrl-single driver with APIs
This patch set adds following feature in pinctrl-single driver,
1. Support to use different register read/write api's based on register width.
2. Parse different gpio properties from dt as part of the probe function. This is required to enable pinctrl pad.
3. add pinctrl_ops->request api to configure pctrl pad register.
Rayagonda Kokatanur (3): drivers: pinctrl-single: handle different register width drivers: pinctrl-single: add support to parse gpio properties drivers: pinctrl-single: add request api
drivers/pinctrl/pinctrl-single.c | 187 +++++++++++++++++++++++++++---- 1 file changed, 163 insertions(+), 24 deletions(-)
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com --- drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/** + * struct single_pdata - pinctrl device instance + * @base first configuration register + * @offset index of last configuration register + * @mask configuration-value mask bits + * @width configuration register bit width + * @bits_per_mux + * @read register read function to use + * @write register write function to use + */ struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux; + u32 (*read)(phys_addr_t reg); + void (*write)(u32 val, phys_addr_t reg); };
struct single_fdt_pin_cfg { @@ -23,6 +35,36 @@ struct single_fdt_pin_cfg { fdt32_t val; /* configuration register value */ };
+static u32 __maybe_unused single_readb(phys_addr_t reg) +{ + return readb(reg); +} + +static u32 __maybe_unused single_readw(phys_addr_t reg) +{ + return readw(reg); +} + +static u32 __maybe_unused single_readl(phys_addr_t reg) +{ + return readl(reg); +} + +static void __maybe_unused single_writeb(u32 val, phys_addr_t reg) +{ + writeb(val, reg); +} + +static void __maybe_unused single_writew(u32 val, phys_addr_t reg) +{ + writew(val, reg); +} + +static void __maybe_unused single_writel(u32 val, phys_addr_t reg) +{ + writel(val, reg); +} + struct single_fdt_bits_cfg { fdt32_t reg; /* configuration register offset */ fdt32_t val; /* configuration register value */ @@ -60,18 +102,9 @@ static int single_configure_pins(struct udevice *dev, } reg += pdata->base; val = fdt32_to_cpu(pins->val) & pdata->mask; - switch (pdata->width) { - case 16: - writew((readw(reg) & ~pdata->mask) | val, reg); - break; - case 32: - writel((readl(reg) & ~pdata->mask) | val, reg); - break; - default: - dev_warn(dev, "unsupported register width %i\n", - pdata->width); - continue; - } + val |= (pdata->read(reg) & ~pdata->mask); + pdata->write(val, reg); + dev_dbg(dev, " reg/val 0x%pa/0x%08x\n", ®, val); } return 0; @@ -97,18 +130,8 @@ static int single_configure_bits(struct udevice *dev, mask = fdt32_to_cpu(pins->mask); val = fdt32_to_cpu(pins->val) & mask;
- switch (pdata->width) { - case 16: - writew((readw(reg) & ~mask) | val, reg); - break; - case 32: - writel((readl(reg) & ~mask) | val, reg); - break; - default: - dev_warn(dev, "unsupported register width %i\n", - pdata->width); - continue; - } + val |= (pdata->read(reg) & ~mask); + pdata->write(val, reg); dev_dbg(dev, " reg/val 0x%pa/0x%08x\n", ®, val); } return 0; @@ -148,6 +171,32 @@ static int single_set_state(struct udevice *dev, return len; }
+static int single_probe(struct udevice *dev) +{ + struct single_pdata *pdata = dev->platdata; + + switch (pdata->width) { + case 8: + pdata->read = single_readb; + pdata->write = single_writeb; + break; + case 16: + pdata->read = single_readw; + pdata->write = single_writew; + break; + case 32: + pdata->read = single_readl; + pdata->write = single_writel; + break; + default: + dev_warn(dev, "%s: unsupported register width %d\n", + __func__, pdata->width); + return -EINVAL; + } + + return 0; +} + static int single_ofdata_to_platdata(struct udevice *dev) { fdt_addr_t addr; @@ -193,4 +242,5 @@ U_BOOT_DRIVER(single_pinctrl) = { .ops = &single_pinctrl_ops, .platdata_auto_alloc_size = sizeof(struct single_pdata), .ofdata_to_platdata = single_ofdata_to_platdata, + .probe = single_probe, };
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);
Can't we just have a read & write function with a switch statement? Why do we need function pointers?
Regards, Simon
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);Can't we just have a read & write function with a switch statement? Why do we need function pointers?
I referred to the linux pinctrl-single.c and kept code similar to linux. Please let me know.
Regards, Simon
Hi Rayagonda,
On Thu, 30 Apr 2020 at 04:06, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);Can't we just have a read & write function with a switch statement? Why do we need function pointers?
I referred to the linux pinctrl-single.c and kept code similar to linux. Please let me know.
See the regmap discussion here which is related:
http://patchwork.ozlabs.org/project/uboot/patch/20191105114700.24989-3-jjhib...
Should this driver use regmap, then?
Regards, Simon
Hi Simon,
On Fri, May 8, 2020 at 7:07 AM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
On Thu, 30 Apr 2020 at 04:06, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);Can't we just have a read & write function with a switch statement? Why do we need function pointers?
I referred to the linux pinctrl-single.c and kept code similar to linux. Please let me know.
See the regmap discussion here which is related:
http://patchwork.ozlabs.org/project/uboot/patch/20191105114700.24989-3-jjhib...
Should this driver use regmap, then?
I think using a function pointer is a better approach, we can check for errors in one place ie invalid register width. Right now it's been done in single_probe() function. Please let me know.
Best regards, Rayagonda
Regards, Simon
Hi Rayagonda,
On Sun, 24 May 2020 at 04:46, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Hi Simon,
On Fri, May 8, 2020 at 7:07 AM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
On Thu, 30 Apr 2020 at 04:06, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);Can't we just have a read & write function with a switch statement? Why do we need function pointers?
I referred to the linux pinctrl-single.c and kept code similar to linux. Please let me know.
See the regmap discussion here which is related:
http://patchwork.ozlabs.org/project/uboot/patch/20191105114700.24989-3-jjhib...
Should this driver use regmap, then?
I think using a function pointer is a better approach, we can check for errors in one place ie invalid register width. Right now it's been done in single_probe() function. Please let me know.
What sort of errors?
I'm sorry but I prefer the switch() for U-Boot. We have different constraints from Linux. After all, our file is 200 lines and in Linux this is 2k lines.
Regards, Simon
Hi Simon,
On Mon, May 25, 2020 at 7:45 AM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
On Sun, 24 May 2020 at 04:46, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Hi Simon,
On Fri, May 8, 2020 at 7:07 AM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
On Thu, 30 Apr 2020 at 04:06, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add support to use different register read/write api's based on register width.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 98 ++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 738f5bd636..aed113b083 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -10,12 +10,24 @@ #include <linux/libfdt.h> #include <asm/io.h>
+/**
- struct single_pdata - pinctrl device instance
- @base first configuration register
- @offset index of last configuration register
- @mask configuration-value mask bits
- @width configuration register bit width
- @bits_per_mux
- @read register read function to use
- @write register write function to use
- */
struct single_pdata { fdt_addr_t base; /* first configuration register */ int offset; /* index of last configuration register */ u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux;
u32 (*read)(phys_addr_t reg);void (*write)(u32 val, phys_addr_t reg);Can't we just have a read & write function with a switch statement? Why do we need function pointers?
I referred to the linux pinctrl-single.c and kept code similar to linux. Please let me know.
See the regmap discussion here which is related:
http://patchwork.ozlabs.org/project/uboot/patch/20191105114700.24989-3-jjhib...
Should this driver use regmap, then?
I think using a function pointer is a better approach, we can check for errors in one place ie invalid register width. Right now it's been done in single_probe() function. Please let me know.
What sort of errors?
What I mean is, right now read/write function pointres are getting initialized in single_probe() based on pdata->width. If pdata->width is invalid, its erroring out there only.
So if we use a single read and write function with switch statement then checking pdata->width should be part of this switch statement. This means every call to read/write should check for failure. Hence I am thinking function pointer is a better approach.
Please let me know.
Best regards, Rayagonda
I'm sorry but I prefer the switch() for U-Boot. We have different constraints from Linux. After all, our file is 200 lines and in Linux this is 2k lines.
Regards, Simon
Parse different gpio properties from dt as part of probe function. This detail will be used to enable pinctrl pad later when gpio lines are requested.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com --- drivers/pinctrl/pinctrl-single.c | 62 +++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index aed113b083..fe79a218ee 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -6,10 +6,26 @@ #include <common.h> #include <dm.h> #include <dm/device_compat.h> +#include <dm/devres.h> +#include <dm/of_access.h> #include <dm/pinctrl.h> #include <linux/libfdt.h> #include <asm/io.h>
+/** + * struct single_gpiofunc_range - pin ranges with same mux value of gpio fun + * @offset: offset base of pins + * @npins: number pins with the same mux value of gpio function + * @gpiofunc: mux value of gpio function + * @node: list node + */ +struct single_gpiofunc_range { + u32 offset; + u32 npins; + u32 gpiofunc; + struct list_head node; +}; + /** * struct single_pdata - pinctrl device instance * @base first configuration register @@ -17,6 +33,8 @@ * @mask configuration-value mask bits * @width configuration register bit width * @bits_per_mux + * @mutex mutex protecting the list + * @gpiofuncs list of gpio functions * @read register read function to use * @write register write function to use */ @@ -26,6 +44,8 @@ struct single_pdata { u32 mask; /* configuration-value mask bits */ int width; /* configuration register bit width */ bool bits_per_mux; + struct mutex mutex; + struct list_head gpiofuncs; u32 (*read)(phys_addr_t reg); void (*write)(u32 val, phys_addr_t reg); }; @@ -171,9 +191,42 @@ static int single_set_state(struct udevice *dev, return len; }
+static int single_add_gpio_func(struct udevice *dev, + struct single_pdata *pdata) +{ + const char *propname = "pinctrl-single,gpio-range"; + const char *cellname = "#pinctrl-single,gpio-range-cells"; + struct single_gpiofunc_range *range; + struct ofnode_phandle_args gpiospec; + int ret, i; + + for (i = 0; ; i++) { + ret = ofnode_parse_phandle_with_args(dev->node, propname, + cellname, 0, i, &gpiospec); + /* Do not treat it as error. Only treat it as end condition. */ + if (ret) { + ret = 0; + break; + } + range = devm_kzalloc(dev, sizeof(*range), GFP_KERNEL); + if (!range) { + ret = -ENOMEM; + break; + } + range->offset = gpiospec.args[0]; + range->npins = gpiospec.args[1]; + range->gpiofunc = gpiospec.args[2]; + mutex_lock(&pdata->mutex); + list_add_tail(&range->node, &pdata->gpiofuncs); + mutex_unlock(&pdata->mutex); + } + return ret; +} + static int single_probe(struct udevice *dev) { struct single_pdata *pdata = dev->platdata; + int ret;
switch (pdata->width) { case 8: @@ -194,7 +247,14 @@ static int single_probe(struct udevice *dev) return -EINVAL; }
- return 0; + mutex_init(&pdata->mutex); + INIT_LIST_HEAD(&pdata->gpiofuncs); + + ret = single_add_gpio_func(dev, pdata); + if (ret < 0) + dev_err(dev, "%s: Failed to add gpio functions\n", __func__); + + return ret; }
static int single_ofdata_to_platdata(struct udevice *dev)
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Parse different gpio properties from dt as part of probe function. This detail will be used to enable pinctrl pad later when gpio lines are requested.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 62 +++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
Can you please add the binding and a test? Also I think this feature should be behind a Kconfig flag to avoid code-size increase.
Regards, Simon
Hi Simon,
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Parse different gpio properties from dt as part of probe function. This detail will be used to enable pinctrl pad later when gpio lines are requested.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 62 +++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
Can you please add the binding and a test? Also I think this feature should be behind a Kconfig flag to avoid code-size increase.
Sorry I didn't get it, please elaborate "binding and a test". You mean dt-binding document and test procedure.
This feature is added by referring to linux pinctrl-single driver and code is in align with linux driver. This feature is going to be used in most of the gpio controllers where they have pin controllers to select different modes of gpio lines. I feel this feature should be part of the driver by default.
Regards, Simon
Hi Rayagonda,
On Thu, 30 Apr 2020 at 05:10, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Hi Simon,
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:35, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Parse different gpio properties from dt as part of probe function. This detail will be used to enable pinctrl pad later when gpio lines are requested.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 62 +++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
Can you please add the binding and a test? Also I think this feature should be behind a Kconfig flag to avoid code-size increase.
Sorry I didn't get it, please elaborate "binding and a test". You mean dt-binding document and test procedure.
That's right. For the tests, it seems we do not have a test/dm/pinctrl.c but we need one, something simple to start.
The binding will help explain what is going on.
This feature is added by referring to linux pinctrl-single driver and code is in align with linux driver. This feature is going to be used in most of the gpio controllers where they have pin controllers to select different modes of gpio lines. I feel this feature should be part of the driver by default.
OK makes sense.
Re the Kconfig flag, U-Boot SPL runs in a tight environment. So when we add new features we sometimes put them behind a flag so that it does not bloat SPL.
Regards, Simon
Add pinctrl_ops->request api to configure pctrl pad register in gpio mode.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com --- drivers/pinctrl/pinctrl-single.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index fe79a218ee..2cdba1d338 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -130,6 +130,34 @@ static int single_configure_pins(struct udevice *dev, return 0; }
+static int single_request(struct udevice *dev, int pin, int flags) +{ + struct single_pdata *pdata = dev->platdata; + struct single_gpiofunc_range *frange = NULL; + struct list_head *pos, *tmp; + int mux_bytes = 0; + u32 data; + + if (!pdata->mask) + return -ENOTSUPP; + + list_for_each_safe(pos, tmp, &pdata->gpiofuncs) { + frange = list_entry(pos, struct single_gpiofunc_range, node); + if ((pin >= frange->offset + frange->npins) || + pin < frange->offset) + continue; + + mux_bytes = pdata->width / BITS_PER_BYTE; + data = pdata->read(pdata->base + pin * mux_bytes); + data &= ~pdata->mask; + data |= frange->gpiofunc; + pdata->write(data, pdata->base + pin * mux_bytes); + break; + } + + return 0; +} + static int single_configure_bits(struct udevice *dev, const struct single_fdt_bits_cfg *pins, int size) @@ -288,6 +316,7 @@ static int single_ofdata_to_platdata(struct udevice *dev)
const struct pinctrl_ops single_pinctrl_ops = { .set_state = single_set_state, + .request = single_request, };
static const struct udevice_id single_pinctrl_match[] = {
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:36, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add pinctrl_ops->request api to configure pctrl pad register in gpio mode.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
This should use the Kconfig also (and needs a test)
Regards, Simon
Hi Simon,
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:36, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add pinctrl_ops->request api to configure pctrl pad register in gpio mode.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
This should use the Kconfig also (and needs a test)
Please elaborate "need a test". Do you mean a testing procedure as part of the commit message ?
I feel we don't need the Kconfig option, looking at Linux pinctrl-single driver. Please let me know.
Regards, Simon
Hi Rayagonda,
On Thu, 30 Apr 2020 at 05:03, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Hi Simon,
On Wed, Apr 29, 2020 at 11:34 PM Simon Glass sjg@chromium.org wrote:
Hi Rayagonda,
+Stephen Warren
On Wed, 29 Apr 2020 at 10:36, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:
Add pinctrl_ops->request api to configure pctrl pad register in gpio mode.
Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com
drivers/pinctrl/pinctrl-single.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
This should use the Kconfig also (and needs a test)
Please elaborate "need a test". Do you mean a testing procedure as part of the commit message ?
No I mean U-Boot has automated tests. So when we add code we need to add tests for that code. See test/dm/rtc.c for an example. You can run them with:
make qcheck
or a single one with:
I feel we don't need the Kconfig option, looking at Linux pinctrl-single driver. Please let me know.
OK we can go without one. It is easy to add later if needed.
Regards, Simon
participants (2)
-
Rayagonda Kokatanur -
Simon Glass