Re: [PATCH] sh-pfc: handle pin array holes in sh_pfc_map_pins()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 05/27/2015 11:52 AM, Geert Uytterhoeven wrote:

The  pin array handled by sh_pfc_map_pins() may contain holes
representing
non- existing pins. We have to first count the valid pins in order to
calculate the size  of the memory to be allocated, then to skip over the
non-existing pins when  initializing the allocated arrays, and then to
return the number of valid pins  from sh_pfc_map_pins() instead of 0 on
success.

As we have to touch devm_kzalloc() calls anyway, use more fitting
devm_kcalloc() instead which additionally checks the array size. And
since
PINMUX_TYPE_NONE is #define'd as 0, stop re-initializing already zeroed
out 'pmx->configs' array.

I agree with this optimization, but I think it deserves a comment in the
source code that explicitly mentions PINMUX_TYPE_NONE, to make sure any
later
rework changing the PINMUX_TYPE_NONE value would catch this.

    Note that this is not just "drove by" optimization, I was trying to avoid
the very need to explicitly initialize 'pmx->configs' array to
PINMUX_TYPE_NONE...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>

[...]

@@ -622,7 +628,7 @@ int sh_pfc_register_pinctrl(struct sh_pf
         pmx->pctl_desc.pmxops = &sh_pfc_pinmux_ops;
         pmx->pctl_desc.confops = &sh_pfc_pinconf_ops;
         pmx->pctl_desc.pins = pmx->pins;
-       pmx->pctl_desc.npins = pfc->info->nr_pins;
+       pmx->pctl_desc.npins = ret;

         pmx->pctl = pinctrl_register(&pmx->pctl_desc, pfc->dev, pmx);
         if (pmx->pctl == NULL)

Shouldn't you also fix sh_pfc_init_ranges() ? It includes the following
code
that doesn't seem to handle holes properly.

          for (i = 1, nr_ranges = 1; i < pfc->info->nr_pins; ++i) {
                  if (pfc->info->pins[i-1].pin != pfc->info->pins[i].pin -
1)
                          nr_ranges++;
          }

Please make sure that sh_pfc_get_pin_index() doesn't start blowing up
afterwards though.

    I think I'm seeing a problem with this function (and it's not due to my
changes). Its result is often used to index 'pfc->info->pins' array. While
this is working now, when the holes are not yet allowed, it's going to break
when we start supporting the holes since that array couldn't be indexed this
way anymore (via ranges). This function is good only for 'pmx->configs' and
the like where we have already removed the holes. It looks like this holes
thing is going to be too complex, so how about leaving things as they are?
:-)

Any conclusion on this?

   I did overestimate the complexity, it seems. :-)
I have just removed the calls to sh_pfc_get_pin_index() on 'pfc->info->pins' and its alias.

If yes, please resend, incl. the r8a7794 pfc patches that depend on it.

   Will repost the patches RSN.

Gr{oetje,eeting}s,
                         Geert

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux