lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Wed, 9 Jul 2014 17:21:37 -0700
From:	Bjorn Andersson <bjorn@...o.se>
To:	fwu@...vell.com
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Tony Lindgren <tony@...mide.com>, plagnioj@...osoft.com,
	Kukjin Kim <kgene.kim@...sung.com>, heiko@...ech.de,
	Tomasz Figa <t.figa@...sung.com>, thomas.abraham@...aro.org,
	srinivas.kandagatla@...il.com, maxime.coquelin@...com,
	patrice.chotard@...com, thierry.reding@...il.com,
	baohua@...nel.org, viresh.linux@...il.com,
	Matt Porter <matt.porter@...aro.org>, syin@...adcom.com,
	jg1.han@...sung.com, Rongjun.Ying@....com, Rong.Wang@....com,
	linux@...sktech.co.nz, yongjun_wei@...ndmicro.com.cn,
	laurent.navet@...il.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Stephen Warren <swarren@...dia.com>, cxie4@...vell.com,
	ylmao@...vell.com, njiang1@...vell.com, tianxf@...vell.com,
	fswu@...vell.com
Subject: Re: [PATCH v6] pinctrl: to avoid duplicated calling
 enable_pinmux_setting for a pin

On Sun, Jun 8, 2014 at 6:37 PM,  <fwu@...vell.com> wrote:
> From: Fan Wu <fwu@...vell.com>
[...]
> diff --git a/drivers/pinctrl/pinctrl-msm.c b/drivers/pinctrl/pinctrl-msm.c
> index df6dda4c..bdfaba4 100644
> --- a/drivers/pinctrl/pinctrl-msm.c
> +++ b/drivers/pinctrl/pinctrl-msm.c
> @@ -165,36 +165,11 @@ static int msm_pinmux_enable(struct pinctrl_dev *pctldev,
>         return 0;
>  }
>
> -static void msm_pinmux_disable(struct pinctrl_dev *pctldev,
> -                              unsigned function,
> -                              unsigned group)
> -{
> -       struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> -       const struct msm_pingroup *g;
> -       unsigned long flags;
> -       u32 val;
> -
> -       g = &pctrl->soc->groups[group];
> -
> -       if (WARN_ON(g->mux_bit < 0))
> -               return;
> -
> -       spin_lock_irqsave(&pctrl->lock, flags);
> -
> -       /* Clear the mux bits to select gpio mode */
> -       val = readl(pctrl->regs + g->ctl_reg);
> -       val &= ~(0x7 << g->mux_bit);
> -       writel(val, pctrl->regs + g->ctl_reg);
> -
> -       spin_unlock_irqrestore(&pctrl->lock, flags);
> -}
> -
>  static const struct pinmux_ops msm_pinmux_ops = {
>         .get_functions_count    = msm_get_functions_count,
>         .get_function_name      = msm_get_function_name,
>         .get_function_groups    = msm_get_function_groups,
>         .enable                 = msm_pinmux_enable,
> -       .disable                = msm_pinmux_disable,
>  };
>

Sorry for being so late to the game. Totally missed when this until Linus today
told me that 'disable' is gone.

Given the following dt snippet:

        active: active {
                foo {
                        pins = "pin1";
                        function = "function";
                };
        };

        sleep: sleep {
                foo {
                        pins = "pin1";
                };
        };

Calling:
        pinctrl_select_state(&active);
        pinctrl_select_state(&sleep);

If I understand the code correctly after your change there will be no pinmux
operations performed during this transition. As you can see from the comment
above the Qualcomm driver relies on this fact to disable muxing; i.e. select
gpio function.

So most likely I have misunderstood what the disable function was supposed to
do, and we would need to add an explicit "gpio" function to all pingroups as
well as explicitly specifying that for every gpio-configured pin in the device
tree.

I.e. the lack of the string 'function = "gpio"' in below example makes the
state incomplete:

foo {
        bar {
                pins = "pin1";
                bias-pull-up;
        };
};

Is this assumption correct?

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists