Re: [PATCH v2 1/2] bus: imx-weim: support multiple address ranges per child node

From: Shawn Guo
Date: Wed Dec 05 2018 - 02:52:35 EST


On Fri, Nov 30, 2018 at 03:56:23PM -0500, thesven73@xxxxxxxxx wrote:
> From: Sven Van Asbroeck <TheSven73@xxxxxxxxxxxxxx>
>
> Ensure that timing values for the child node are applied to
> all chip selects in the child's address ranges.
>
> Note that this does not support multiple timing settings per
> child; this can be added in the future if required.
>
> Example:
> &weim {
> acme@0,0 {
> compatible = "acme,whatever";
> reg = <0 0 0x100>, <0 0x400000 0x800>,
> <1 0x400000 0x800>;
> fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
> 0x00000000 0xa0000240 0x00000000>;
> };
> };

I'm not sure about that. Shouldn't we have another child node for
different chip select, something like below?

&weim {
acme@0,0 {
compatible = "acme,whatever";
reg = <0 0 0x100>, <0 0x400000 0x800>;
fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
0x00000000 0xa0000240 0x00000000>;
};

acme@1,400000 {
compatible = "acme,whatever";
reg = <1 0x400000 0x800>;
fsl,weim-cs-timing = <0x024400b1 0x00001010 0x20081100
0x00000000 0xa0000240 0x00000000>;
};

Shawn

>
> Signed-off-by: Sven Van Asbroeck <TheSven73@xxxxxxxxxxxxxx>
> ---
> drivers/bus/imx-weim.c | 36 +++++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
> index f01308172de9..24f22285395d 100644
> --- a/drivers/bus/imx-weim.c
> +++ b/drivers/bus/imx-weim.c
> @@ -46,6 +46,7 @@ static const struct imx_weim_devtype imx51_weim_devtype = {
> };
>
> #define MAX_CS_REGS_COUNT 6
> +#define OF_REG_SIZE 3
>
> static const struct of_device_id weim_id_table[] = {
> /* i.MX1/21 */
> @@ -115,27 +116,40 @@ static int __init weim_timing_setup(struct device_node *np, void __iomem *base,
> const struct imx_weim_devtype *devtype)
> {
> u32 cs_idx, value[MAX_CS_REGS_COUNT];
> - int i, ret;
> + int i, ret, reg_idx, num_regs;
>
> if (WARN_ON(devtype->cs_regs_count > MAX_CS_REGS_COUNT))
> return -EINVAL;
>
> - /* get the CS index from this child node's "reg" property. */
> - ret = of_property_read_u32(np, "reg", &cs_idx);
> + ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
> + value, devtype->cs_regs_count);
> if (ret)
> return ret;
>
> - if (cs_idx >= devtype->cs_count)
> + /*
> + * the child node's "reg" property may contain multiple address ranges,
> + * extract the chip select for each.
> + */
> + num_regs = of_property_count_elems_of_size(np, "reg", OF_REG_SIZE);
> + if (num_regs < 0)
> + return num_regs;
> + if (!num_regs)
> return -EINVAL;
> + for (reg_idx = 0; reg_idx < num_regs; reg_idx++) {
> + /* get the CS index from this child node's "reg" property. */
> + ret = of_property_read_u32_index(np, "reg",
> + reg_idx*OF_REG_SIZE, &cs_idx);
> + if (ret)
> + break;
>
> - ret = of_property_read_u32_array(np, "fsl,weim-cs-timing",
> - value, devtype->cs_regs_count);
> - if (ret)
> - return ret;
> + if (cs_idx >= devtype->cs_count)
> + return -EINVAL;
>
> - /* set the timing for WEIM */
> - for (i = 0; i < devtype->cs_regs_count; i++)
> - writel(value[i], base + cs_idx * devtype->cs_stride + i * 4);
> + /* set the timing for WEIM */
> + for (i = 0; i < devtype->cs_regs_count; i++)
> + writel(value[i],
> + base + cs_idx * devtype->cs_stride + i * 4);
> + }
>
> return 0;
> }
> --
> 2.17.1
>