Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1720500 > unrolled thread

Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Stephen Boyd <sboyd@codeaurora.org> - 2017-08-26 01:20 +0200
    Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Shawn Guo <shawnguo@kernel.org> - 2017-08-26 05:50 +0200
      Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Stephen Boyd <sboyd@codeaurora.org> - 2017-08-30 23:10 +0200
        Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Shawn Guo <shawnguo@kernel.org> - 2017-08-31 10:40 +0200
          Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Stephen Boyd <sboyd@codeaurora.org> - 2017-09-01 03:40 +0200
            Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally Shawn Guo <shawnguo@kernel.org> - 2017-09-01 05:10 +0200

#1720500 — Re: [PATCH V2] spmi: pmic-arb: Enforce the ownership check optionally

On 08/25, Shawn Guo wrote:
> On Thu, Aug 24, 2017 at 11:37:01AM -0700, Stephen Boyd wrote:
> > On 08/24, Shawn Guo wrote:
> > > On Tue, Aug 22, 2017 at 01:31:32PM -0700, Stephen Boyd wrote:
> > > > Also, I see that on v4.13-rc series the read/write checks are
> > > > causing the led driver to fail in a different way:
> > > > 
> > > >     spmi spmi-0: error: impermissible write to peripheral sid:0 addr:0xc040
> > > >     qcom-spmi-gpio 200f000.spmi:pm8916@0:gpios@c000: write 0x40 failed
> > > >     leds-gpio soc:leds: Error applying setting, reverse things back
> > > >     spmi spmi-0: error: impermissible write to peripheral sid:0 addr:0xc041
> > > >     qcom-spmi-gpio 200f000.spmi:pm8916@0:gpios@c000: write 0x41 failed
> > > >     leds-gpio: probe of soc:leds failed with error -1 
> > > > 
> > > > Are you seeing similar behavior?
> > > 
> > > Yes.  I forgot to mention that, and leds-gpio failure is gone after
> > > applying Kiran's patch below.
> > > 
> > > spmi: pmic-arb: remove the read/write access checks
> > > 
> > 
> > Sure. Removing the checks will silence the warnings, but it still
> > means that we're attempting to configure GPIOs that we shouldn't
> > be configuring.
> 
> The driver is attempting to configure the GPIOs that device tree tells
> to.
> 
> 	led@3 {                        
> 		label = "apq8016-sbc:green:user3";
> 		gpios = <&pm8916_gpios 1 GPIO_ACTIVE_HIGH>;
> 		linux,default-trigger = "mmc1";
> 		default-state = "off";         
> 	};
> 
> Are you saying, in case of user3 led above, device tree shouldn't use
> GPIO <&pm8916_gpios 1> there at all?

Right. Does the GPIO work? If so, it sounds like the read/write
access checks in spmi pmic arb don't work properly.

> 
> > Is there some sort of default configuration that
> > gets applied to all pins by default?
> 
> I do not quite understand what you are asking and how that is related to
> the thing we discuss here.  But my understanding is that spmi_arb
> read/write access is used not only by pinctrl API to set up pinmux for
> GPIO function, but also by GPIO API to actually drive the GPIO.
> 

Ah I got confused because I thought we numbered GPIO pins from 0,
but on the PMIC we number from 1. I see that base = -1 assignment
now. I thought that all pins on the pmic were being configured
somewhere because I didn't see a gpio 0 usage in DT.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1720532

On Fri, Aug 25, 2017 at 04:18:18PM -0700, Stephen Boyd wrote:
> On 08/25, Shawn Guo wrote:
> > On Thu, Aug 24, 2017 at 11:37:01AM -0700, Stephen Boyd wrote:
> > > On 08/24, Shawn Guo wrote:
> > > > On Tue, Aug 22, 2017 at 01:31:32PM -0700, Stephen Boyd wrote:
> > > > > Also, I see that on v4.13-rc series the read/write checks are
> > > > > causing the led driver to fail in a different way:
> > > > > 
> > > > >     spmi spmi-0: error: impermissible write to peripheral sid:0 addr:0xc040
> > > > >     qcom-spmi-gpio 200f000.spmi:pm8916@0:gpios@c000: write 0x40 failed
> > > > >     leds-gpio soc:leds: Error applying setting, reverse things back
> > > > >     spmi spmi-0: error: impermissible write to peripheral sid:0 addr:0xc041
> > > > >     qcom-spmi-gpio 200f000.spmi:pm8916@0:gpios@c000: write 0x41 failed
> > > > >     leds-gpio: probe of soc:leds failed with error -1 
> > > > > 
> > > > > Are you seeing similar behavior?
> > > > 
> > > > Yes.  I forgot to mention that, and leds-gpio failure is gone after
> > > > applying Kiran's patch below.
> > > > 
> > > > spmi: pmic-arb: remove the read/write access checks
> > > > 
> > > 
> > > Sure. Removing the checks will silence the warnings, but it still
> > > means that we're attempting to configure GPIOs that we shouldn't
> > > be configuring.
> > 
> > The driver is attempting to configure the GPIOs that device tree tells
> > to.
> > 
> > 	led@3 {                        
> > 		label = "apq8016-sbc:green:user3";
> > 		gpios = <&pm8916_gpios 1 GPIO_ACTIVE_HIGH>;
> > 		linux,default-trigger = "mmc1";
> > 		default-state = "off";         
> > 	};
> > 
> > Are you saying, in case of user3 led above, device tree shouldn't use
> > GPIO <&pm8916_gpios 1> there at all?
> 
> Right. Does the GPIO work? If so, it sounds like the read/write
> access checks in spmi pmic arb don't work properly.

The check works.  With the check in there, PM8916 GPIO doesn't work.
However, the consequence is that not only user3 but all GPIO leds under
'leds' node will fail to register, because any GPIO led's failing on
create_gpio_led() makes leds-gpio driver probe fail as a while.  That's
how leds-gpio driver works.

Also, per schematics, PM8916 GPIO1 is indeed routed to user3 LED on
db410c board.  Why do you think apq8016-sbc device tree shouldn't use
the GPIO for that at all?  Isn't it firmware's fault that the ownership
of the peripheral is not properly configured?

Shawn

[toc] | [prev] | [next] | [standalone]

#1723664

On 08/26, Shawn Guo wrote:
> On Fri, Aug 25, 2017 at 04:18:18PM -0700, Stephen Boyd wrote:
> > 
> > Right. Does the GPIO work? If so, it sounds like the read/write
> > access checks in spmi pmic arb don't work properly.
> 
> The check works.  With the check in there, PM8916 GPIO doesn't work.
> However, the consequence is that not only user3 but all GPIO leds under
> 'leds' node will fail to register, because any GPIO led's failing on
> create_gpio_led() makes leds-gpio driver probe fail as a while.  That's
> how leds-gpio driver works.
> 
> Also, per schematics, PM8916 GPIO1 is indeed routed to user3 LED on
> db410c board.  Why do you think apq8016-sbc device tree shouldn't use
> the GPIO for that at all?  Isn't it firmware's fault that the ownership
> of the peripheral is not properly configured?

If the ownership was not properly configured in the firmware,
then I imagine it would mean that we can't control the GPIO for
the LED. But that doesn't seem to be true. I can see on my board
that I get impermissible write failures on the GPIO when
controlling the GPIO brightness, but it doesn't actually matter
because the led still lights up. So the checks for write/read
permission seem incorrect, or they're not being enforced.

Anyway, I just wanted to make sure the GPIO still works, and it
looks like it does, so removing the permission checks is enough
to make me happy.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1724003

On Wed, Aug 30, 2017 at 02:02:03PM -0700, Stephen Boyd wrote:
> On 08/26, Shawn Guo wrote:
> > On Fri, Aug 25, 2017 at 04:18:18PM -0700, Stephen Boyd wrote:
> > > 
> > > Right. Does the GPIO work? If so, it sounds like the read/write
> > > access checks in spmi pmic arb don't work properly.
> > 
> > The check works.  With the check in there, PM8916 GPIO doesn't work.
> > However, the consequence is that not only user3 but all GPIO leds under
> > 'leds' node will fail to register, because any GPIO led's failing on
> > create_gpio_led() makes leds-gpio driver probe fail as a while.  That's
> > how leds-gpio driver works.
> > 
> > Also, per schematics, PM8916 GPIO1 is indeed routed to user3 LED on
> > db410c board.  Why do you think apq8016-sbc device tree shouldn't use
> > the GPIO for that at all?  Isn't it firmware's fault that the ownership
> > of the peripheral is not properly configured?
> 
> If the ownership was not properly configured in the firmware,
> then I imagine it would mean that we can't control the GPIO for
> the LED. But that doesn't seem to be true. I can see on my board
> that I get impermissible write failures on the GPIO when
> controlling the GPIO brightness, but it doesn't actually matter
> because the led still lights up. So the checks for write/read
> permission seem incorrect, or they're not being enforced.

I'm not sure what is happening on your side.  As I said above, with the
4.13-rc series, leds-gpio driver doesn't probe at all, due to the
impermissible write to PM8916 GPIO in function create_gpio_led(), and
none of the LEDs lights up on my board.

Shawn

[toc] | [prev] | [next] | [standalone]

#1724717

On 08/31, Shawn Guo wrote:
> On Wed, Aug 30, 2017 at 02:02:03PM -0700, Stephen Boyd wrote:
> > On 08/26, Shawn Guo wrote:
> > > On Fri, Aug 25, 2017 at 04:18:18PM -0700, Stephen Boyd wrote:
> > > > 
> > > > Right. Does the GPIO work? If so, it sounds like the read/write
> > > > access checks in spmi pmic arb don't work properly.
> > > 
> > > The check works.  With the check in there, PM8916 GPIO doesn't work.
> > > However, the consequence is that not only user3 but all GPIO leds under
> > > 'leds' node will fail to register, because any GPIO led's failing on
> > > create_gpio_led() makes leds-gpio driver probe fail as a while.  That's
> > > how leds-gpio driver works.
> > > 
> > > Also, per schematics, PM8916 GPIO1 is indeed routed to user3 LED on
> > > db410c board.  Why do you think apq8016-sbc device tree shouldn't use
> > > the GPIO for that at all?  Isn't it firmware's fault that the ownership
> > > of the peripheral is not properly configured?
> > 
> > If the ownership was not properly configured in the firmware,
> > then I imagine it would mean that we can't control the GPIO for
> > the LED. But that doesn't seem to be true. I can see on my board
> > that I get impermissible write failures on the GPIO when
> > controlling the GPIO brightness, but it doesn't actually matter
> > because the led still lights up. So the checks for write/read
> > permission seem incorrect, or they're not being enforced.
> 
> I'm not sure what is happening on your side.  As I said above, with the
> 4.13-rc series, leds-gpio driver doesn't probe at all, due to the
> impermissible write to PM8916 GPIO in function create_gpio_led(), and
> none of the LEDs lights up on my board.
> 

Yep. I understand all that.

Sorry, I forgot to mention I modified the SPMI PMIC arb code on
v4.13-rc7 to continue even though a permission fault may happen
by deleting the 'return -EPERM' lines. So the LED GPIO driver is
still probing for me, and I see that the GPIOs work regardless of
any permission problems that may have been enforced in the
hardware. I thought the permission checks that the software is
looking at to return EPERM were enforced in hardware, but that
doesn't seem to be the case. That's all I was wondering about.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]

#1724758

On Thu, Aug 31, 2017 at 06:30:48PM -0700, Stephen Boyd wrote:
> On 08/31, Shawn Guo wrote:
> > On Wed, Aug 30, 2017 at 02:02:03PM -0700, Stephen Boyd wrote:
> > > On 08/26, Shawn Guo wrote:
> > > > On Fri, Aug 25, 2017 at 04:18:18PM -0700, Stephen Boyd wrote:
> > > > > 
> > > > > Right. Does the GPIO work? If so, it sounds like the read/write
> > > > > access checks in spmi pmic arb don't work properly.
> > > > 
> > > > The check works.  With the check in there, PM8916 GPIO doesn't work.
> > > > However, the consequence is that not only user3 but all GPIO leds under
> > > > 'leds' node will fail to register, because any GPIO led's failing on
> > > > create_gpio_led() makes leds-gpio driver probe fail as a while.  That's
> > > > how leds-gpio driver works.
> > > > 
> > > > Also, per schematics, PM8916 GPIO1 is indeed routed to user3 LED on
> > > > db410c board.  Why do you think apq8016-sbc device tree shouldn't use
> > > > the GPIO for that at all?  Isn't it firmware's fault that the ownership
> > > > of the peripheral is not properly configured?
> > > 
> > > If the ownership was not properly configured in the firmware,
> > > then I imagine it would mean that we can't control the GPIO for
> > > the LED. But that doesn't seem to be true. I can see on my board
> > > that I get impermissible write failures on the GPIO when
> > > controlling the GPIO brightness, but it doesn't actually matter
> > > because the led still lights up. So the checks for write/read
> > > permission seem incorrect, or they're not being enforced.
> > 
> > I'm not sure what is happening on your side.  As I said above, with the
> > 4.13-rc series, leds-gpio driver doesn't probe at all, due to the
> > impermissible write to PM8916 GPIO in function create_gpio_led(), and
> > none of the LEDs lights up on my board.
> > 
> 
> Yep. I understand all that.
> 
> Sorry, I forgot to mention I modified the SPMI PMIC arb code on
> v4.13-rc7 to continue even though a permission fault may happen
> by deleting the 'return -EPERM' lines. So the LED GPIO driver is
> still probing for me, and I see that the GPIOs work regardless of
> any permission problems that may have been enforced in the
> hardware. I thought the permission checks that the software is
> looking at to return EPERM were enforced in hardware, but that
> doesn't seem to be the case. That's all I was wondering about.

Ah, okay.  You were asking about the check in hardware, while I was
talking about the check in software.  Yes, now we are on the same page:
the permission check in hardware seems not enforced.  That's why LED
works after we merely remove the check in SPMI PMIC arb driver code.

Shawn

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web