Hi Phil,
On Mon, Oct 15, 2018 at 04:01:47PM +0100, Phil Edworthy wrote:
> This fixes the check for unused mdio bus setting and the following static
> checker warning:
> drivers/pinctrl/pinctrl-rzn1.c:198 rzn1_pinctrl_mdio_select()
> warn: always true condition '(ipctl->mdio_func[mdio] >= 0) => (0-u32max >= 0)'
>
> It also fixes the return var when calling of_get_child_count()
>
Not really, since you skip the assignement if return value is <= 0:
nfuncs = of_get_child_count(np);
if (nfuncs <= 0)
return 0;
ipctl->nfunctions = nfuncs;
This seems more likely to be here to make 'rzn1_pmx_get_funcs_count()'
happy, as it returns a signed integer, but since nfunctions is only
assigned if 'nfuncs' > 0, then the cast is safe there.
I would keep this unsigned, and rather return an error if
'of_get_child_count()' returns an error of any sort in 'probe_dt()',
otherwise assign 'ipctl->nfunctions = nfuncs;' unconditionally and
return immediately if nfuncs == 0. This makes sure nfunctions is
initialized even if there are no functions registered.
Anyway, small issues, this just doesn't belong to this patch, but it's
likely not to cause any harm I guess.
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> ---
> v2:
> - Don't use implicit type conversion.
> - Fix type of return var when calling of_get_child_count().
> ---
> drivers/pinctrl/pinctrl-rzn1.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c
> index ce05e3a00be2..4998463c54a0 100644
> --- a/drivers/pinctrl/pinctrl-rzn1.c
> +++ b/drivers/pinctrl/pinctrl-rzn1.c
> @@ -112,13 +112,13 @@ struct rzn1_pinctrl {
> struct rzn1_pinctrl_regs __iomem *lev2;
> u32 lev1_protect_phys;
> u32 lev2_protect_phys;
> - u32 mdio_func[2];
> + int mdio_func[2];
>
> struct rzn1_pin_group *groups;
> unsigned int ngroups;
>
> struct rzn1_pmx_func *functions;
> - unsigned int nfunctions;
> + int nfunctions;
> };
>
> #define RZN1_PINS_PROP "pinmux"
> @@ -195,9 +195,9 @@ static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
> static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, int mdio,
> u32 func)
> {
> - if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
> + if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != (int)func)
> dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
> - ipctl->mdio_func[mdio] = func;
> + ipctl->mdio_func[mdio] = (int)func;
>
> dev_dbg(ipctl->dev, "setting mdio%d to %u\n", mdio, func);
>
> @@ -810,8 +810,8 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
> struct device_node *np = pdev->dev.of_node;
> struct device_node *child;
> unsigned int maxgroups = 0;
> - unsigned int nfuncs = 0;
> unsigned int i = 0;
> + int nfuncs = 0;
> int ret;
>
> nfuncs = of_get_child_count(np);
> --
> 2.17.1
>
Attachment:
signature.asc
Description: PGP signature
