Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping

[Linus Walleij]

On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin  wrote:

> Without such a range, gpiolib fails with -EPROBE_DEFER, pending the
> addition of the range. So, without a range, gpiolib will keep
> deferring indefinitely.
>
> Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond 
> sleep")
> Suggested-by: Linus Walleij 
> Signed-off-by: Peter Rosin 

Added the other Fixes: tag as well, add Cc: stable and applied.

It will go in for the merge window as it is now, unless Torvalds
decide to do an -rc9 in which case I will consider sending it as
a separate fix next week.

Yours,
Linus Walleij

Re: [PATCH 3/3] pinctrl: sx150x: add a static gpio/pinctrl pin range mapping

[Linus Walleij]

On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin  wrote:

> Without such a range, gpiolib fails with -EPROBE_DEFER, pending the
> addition of the range. So, without a range, gpiolib will keep
> deferring indefinitely.
>
> Fixes: e10f72bf4b3e ("gpio: gpiolib: Generalise state persistence beyond 
> sleep")
> Suggested-by: Linus Walleij 
> Signed-off-by: Peter Rosin 

Added the other Fixes: tag as well, add Cc: stable and applied.

It will go in for the merge window as it is now, unless Torvalds
decide to do an -rc9 in which case I will consider sending it as
a separate fix next week.

Yours,
Linus Walleij

Re: [RFC PATCH 1/2] pinctrl: add consumer variant for gpio request

[Ludovic Desroches]

On Wed, Jan 17, 2018 at 06:07:02PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 17, 2018 at 4:54 PM, Ludovic Desroches
>  wrote:
> > On Tue, Jan 16, 2018 at 04:33:29PM +0200, Andy Shevchenko wrote:
> 
> >> First of all, the main architectural issue with current pin control
> >> design is so called "states". It's quite artificial entity which is
> >> not represented by hardware per se.
> >>
> >> Instead we better to provide an API to pin configuration and pin
> >> muxing: I would like to switch to *this* pin function with *this* pin
> >> configuration.
> 
> > gpio_request_enable() allows to switch to the GPIO pin function.
> 
> ...as a particular case of "set this pin to this function".
> 
> > To clarify the situation, from my point of view there are two issues:
> >
> > - Several pin controllers didn't enabled the strict mode when they were
> >   introduced. Nowadays, enabling it will break several DTs. Maybe a DT
> >   property to enable/disable strict mode could do the trick: we remove
> >   pinctrl nodes (so pinmux + pinconf) for pins which will be requested
> >   by gpiod_request() and we set the strict property to true.
> 
> OK.
> 
> > - But... The GPIO pin configuration is missing.
> 
> Here you mixed up the things. Either we are talking about GPIO
> configuration (direction, enabling/disabling I/O buffers), or we are
> talking about pin configuration (bias, drive strength, slew rate,
> debounce, etc.).

I was talking about the pin configuration of the GPIO so about bias,
drive strength, etc.

> 
> >  Some configuration features
> >   have been added, we can continue to add new ones but I am not sure
> >   it's the best solution.
> 
> See below.
> 
> >  At the moment, we rely on a single cell to
> >   manage the configuration. It may not be enough and each time a new pin
> >   configuration will appear, it will have to be added both to the gpiolib
> >   and pinconf.
> 
> >> Second, the pin control and GPIOs are orthogonal as your hardware
> >> confirms. The propagating pin configuration or muxing via GPIO API
> >> looks to me a wrong direction.
> >>
> >
> > I agree but it seems we have started to follow this path.
> 
> Which is still wrong and nothing prevents us to just stop here and
> consider better way.
> 
> The issue is how we historically represent pins inside kernel and how
> hardware evolves at the same time.
> 
> Looking from now retrospectively I can state the following:
> - each GPIO controller *might* have (few) capabilities of pin configuration
> - pin control may not necessary have GPIO function of the pin
> 
> Design is now GPIO oriented while it should be pin oriented.
> 

Agree

> So, instead of littering GPIO API, would we consider to redesign a bit
> from the above point of view?
> 
> (Read ahead: to be backward compatible and be more friendly for simple
> GPIO IPs it would make sense to leave some of the basic pin properties
> to be controlled from GPIO API, otherwise forget completely about GPIO
> driver, and think of pin control driver for new complex IPs)
> 
> [pin]: might have attached set of functions, set of [electrical] properties.
> It might be re-configured run time in terms of function and configuration.
> It might have none, one, or more owners (this is already an OS abstraction)
> 
> Thus, the core function is pin request, while GPIO request is just a
> particular case.
> Owner of the pin is defined independently on what function or
> configuration is chosen.
> 
> Therefore, we will have something like this (permissions):
> - none (no one can do anything, except acquiring an ownership == requesting 
> pin)
> - exclusive (only one user allowed to cover all properties of the pin)
> - shared (several owners can do modify all or selected properties)
> - shared for all (anyone can do anything, perhaps we don't need this)
> 

It seems nice.

> >> To the point of the specific issue you are trying to solve.  I would
> >> rather agree with Maxime:
> >>
> >> "Something like "if the configuration is a gpio, and my pinctrl driver
> >> is also a gpio controller, and that gpiod_request is being called on
> >> that pin, hand over the reference"
> 
> > Sorry, what do you see behind "hand over the reference"?
> 
> This is similar to what I called before as "shared" ownership. To be
> precise, transformation "exclusive" to "shared".
> 
> >> Just in case of architectural review imagine a below case. We have two
> >> UART devices which share same pins, and at the same time their pins
> >> can be switched to GPIO function. So, use cases and questions:
> >> - allow potential driver to switch between UARTs at run time
> >> - allow UART driver to switch mode between no flow control (2 wire
> >> mode) and HW flow (4 wire mode), though not specifically at run time
> >> - allow GPIO function for some pins at run time to support OOB wake
> >> up, for example, when UART is a console
> >>
> >
> > I have the feeling that 1 and 2 are achievable with pinctrl states. 3 

Re: [RFC PATCH 1/2] pinctrl: add consumer variant for gpio request

[Ludovic Desroches]

On Wed, Jan 17, 2018 at 06:07:02PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 17, 2018 at 4:54 PM, Ludovic Desroches
>  wrote:
> > On Tue, Jan 16, 2018 at 04:33:29PM +0200, Andy Shevchenko wrote:
> 
> >> First of all, the main architectural issue with current pin control
> >> design is so called "states". It's quite artificial entity which is
> >> not represented by hardware per se.
> >>
> >> Instead we better to provide an API to pin configuration and pin
> >> muxing: I would like to switch to *this* pin function with *this* pin
> >> configuration.
> 
> > gpio_request_enable() allows to switch to the GPIO pin function.
> 
> ...as a particular case of "set this pin to this function".
> 
> > To clarify the situation, from my point of view there are two issues:
> >
> > - Several pin controllers didn't enabled the strict mode when they were
> >   introduced. Nowadays, enabling it will break several DTs. Maybe a DT
> >   property to enable/disable strict mode could do the trick: we remove
> >   pinctrl nodes (so pinmux + pinconf) for pins which will be requested
> >   by gpiod_request() and we set the strict property to true.
> 
> OK.
> 
> > - But... The GPIO pin configuration is missing.
> 
> Here you mixed up the things. Either we are talking about GPIO
> configuration (direction, enabling/disabling I/O buffers), or we are
> talking about pin configuration (bias, drive strength, slew rate,
> debounce, etc.).

I was talking about the pin configuration of the GPIO so about bias,
drive strength, etc.

> 
> >  Some configuration features
> >   have been added, we can continue to add new ones but I am not sure
> >   it's the best solution.
> 
> See below.
> 
> >  At the moment, we rely on a single cell to
> >   manage the configuration. It may not be enough and each time a new pin
> >   configuration will appear, it will have to be added both to the gpiolib
> >   and pinconf.
> 
> >> Second, the pin control and GPIOs are orthogonal as your hardware
> >> confirms. The propagating pin configuration or muxing via GPIO API
> >> looks to me a wrong direction.
> >>
> >
> > I agree but it seems we have started to follow this path.
> 
> Which is still wrong and nothing prevents us to just stop here and
> consider better way.
> 
> The issue is how we historically represent pins inside kernel and how
> hardware evolves at the same time.
> 
> Looking from now retrospectively I can state the following:
> - each GPIO controller *might* have (few) capabilities of pin configuration
> - pin control may not necessary have GPIO function of the pin
> 
> Design is now GPIO oriented while it should be pin oriented.
> 

Agree

> So, instead of littering GPIO API, would we consider to redesign a bit
> from the above point of view?
> 
> (Read ahead: to be backward compatible and be more friendly for simple
> GPIO IPs it would make sense to leave some of the basic pin properties
> to be controlled from GPIO API, otherwise forget completely about GPIO
> driver, and think of pin control driver for new complex IPs)
> 
> [pin]: might have attached set of functions, set of [electrical] properties.
> It might be re-configured run time in terms of function and configuration.
> It might have none, one, or more owners (this is already an OS abstraction)
> 
> Thus, the core function is pin request, while GPIO request is just a
> particular case.
> Owner of the pin is defined independently on what function or
> configuration is chosen.
> 
> Therefore, we will have something like this (permissions):
> - none (no one can do anything, except acquiring an ownership == requesting 
> pin)
> - exclusive (only one user allowed to cover all properties of the pin)
> - shared (several owners can do modify all or selected properties)
> - shared for all (anyone can do anything, perhaps we don't need this)
> 

It seems nice.

> >> To the point of the specific issue you are trying to solve.  I would
> >> rather agree with Maxime:
> >>
> >> "Something like "if the configuration is a gpio, and my pinctrl driver
> >> is also a gpio controller, and that gpiod_request is being called on
> >> that pin, hand over the reference"
> 
> > Sorry, what do you see behind "hand over the reference"?
> 
> This is similar to what I called before as "shared" ownership. To be
> precise, transformation "exclusive" to "shared".
> 
> >> Just in case of architectural review imagine a below case. We have two
> >> UART devices which share same pins, and at the same time their pins
> >> can be switched to GPIO function. So, use cases and questions:
> >> - allow potential driver to switch between UARTs at run time
> >> - allow UART driver to switch mode between no flow control (2 wire
> >> mode) and HW flow (4 wire mode), though not specifically at run time
> >> - allow GPIO function for some pins at run time to support OOB wake
> >> up, for example, when UART is a console
> >>
> >
> > I have the feeling that 1 and 2 are achievable with pinctrl states. 3 may be
> > also acheviable 

Re: [PATCH v5 11/44] clk: davinci: Add platform information for TI DA830 PSC

[Sekhar Nori]

On Wednesday 17 January 2018 11:02 PM, David Lechner wrote:
> +    clk_register_clkdev(clk_data->clks[3], "gpio", NULL);

 This is pretty bad (and no fault of yours) - having a con_id but no
 device name. Can you please make a pre-series which passes NULL con_id
 in gpio-davinci.c?
>>>
>>> I'll give it a try. This is complicated by the fact that the con_id has
>>> made it's way into the device tree bindings. However, I think we can
>>> safely deprecate clock-names = "gpio" in the device tree bindings since
>>> we can make the driver ignore that property to preserve backwards
>>> compatibility.
> 
> Agreed.
> 
>> I don't think this breaks DT-backward compatibility. Passing a NULL
>> con_id in driver should find the clock for that device even if DT entry
>> has clock-names present. As far as I can read clk_find().
>>
>> The less intrusive alternate is to add the GPIO device name in the table
>> here, while keeping the con_id and keeping the driver untouched. The
>> advantage of that is lesser number of dependent patches for this series
>> to go in.
>>
>> Later once CCF conversion has been there in the kernel for one full
>> release and no regressions, these other clean-ups can be done.
> 
> I like this approach.

One downside is that we will have to have clock-names = "gpio" in da850
device-tree too. Since its already present in keystone already, I don't
think adding one more is such a big issue.

Thanks,
Sekhar

Re: [PATCH v5 11/44] clk: davinci: Add platform information for TI DA830 PSC

[Sekhar Nori]

On Wednesday 17 January 2018 11:02 PM, David Lechner wrote:
> +    clk_register_clkdev(clk_data->clks[3], "gpio", NULL);

 This is pretty bad (and no fault of yours) - having a con_id but no
 device name. Can you please make a pre-series which passes NULL con_id
 in gpio-davinci.c?
>>>
>>> I'll give it a try. This is complicated by the fact that the con_id has
>>> made it's way into the device tree bindings. However, I think we can
>>> safely deprecate clock-names = "gpio" in the device tree bindings since
>>> we can make the driver ignore that property to preserve backwards
>>> compatibility.
> 
> Agreed.
> 
>> I don't think this breaks DT-backward compatibility. Passing a NULL
>> con_id in driver should find the clock for that device even if DT entry
>> has clock-names present. As far as I can read clk_find().
>>
>> The less intrusive alternate is to add the GPIO device name in the table
>> here, while keeping the con_id and keeping the driver untouched. The
>> advantage of that is lesser number of dependent patches for this series
>> to go in.
>>
>> Later once CCF conversion has been there in the kernel for one full
>> release and no regressions, these other clean-ups can be done.
> 
> I like this approach.

One downside is that we will have to have clock-names = "gpio" in da850
device-tree too. Since its already present in keystone already, I don't
think adding one more is such a big issue.

Thanks,
Sekhar

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

[Chen-Yu Tsai]

On Thu, Jan 18, 2018 at 3:22 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
>> > if (sun4i_drv_node_is_connector(node))
>> > return 0;
>> >
>> > -   if (!sun4i_drv_node_is_frontend(node)) {
>> > +   /*
>> > +* If the device is either just a regular device, or an
>> > +* enabled frontend supported by the driver, we add it to our
>> > +* component list.
>> > +*/
>> > +   if (!sun4i_drv_node_is_frontend(node) ||
>> > +   (sun4i_drv_node_is_supported_frontend(node) &&
>> > +of_device_is_available(node))) {
>>
>> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
>> sun4i_drv_node_is_frontend, so of_device_is_available should always
>> be true at this point.
>
> That's not really the condition though :)
>
> It's if the device is *not* a frontend or if it is a supported
> frontend that is available, add it to the endpoints list.

Right. I got confused by the inverted logic. Sorry.

>
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
>> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
>> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
>>
>> Seems that you also need to set the "ALPHA_EN" bit for ARGB.
>
> I have not seen that bit documented anywhere. Where is it coming from?

The A31's user manual. I just checked all the datasheets, and only
the A31 and the A80 have this bit (bit 7) defined. It says if the
bit is cleared, alpha is replaced with 0xff. I assume it works either
way on the A33, or you wouldn't be suprised. Leave it out for now then.
(Or maybe a TODO note now that we know about it.)

ChenYu

>
>> > +   frontend->reset = devm_reset_control_get(dev, NULL);
>> > +   if (IS_ERR(frontend->reset)) {
>> > +   dev_err(dev, "Couldn't get our reset line\n");
>> > +   return PTR_ERR(frontend->reset);
>> > +   }
>> > +   reset_control_reset(frontend->reset);
>>
>> reset_control_reset leaves the reset control deasserted. At this
>> point the clock might not be running, which might mean the internal
>> state is not completely wiped out. (Though this really depends on
>> the design of the internal logic.)
>>
>> Maybe just assert it? It gets deasserted in the runtime PM callback
>> later. And just to be safe, I would move it close to the end of the
>> probe path, past all possible errors, so the hardware doesn't get
>> touched until everything is ready. Or don't touch it anywhere in
>> the probe path, and have the runtime PM resume function do a reset.
>
> That seems like the best solution yes.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

[Chen-Yu Tsai]

On Thu, Jan 18, 2018 at 3:22 PM, Maxime Ripard
 wrote:
> Hi,
>
> On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
>> > if (sun4i_drv_node_is_connector(node))
>> > return 0;
>> >
>> > -   if (!sun4i_drv_node_is_frontend(node)) {
>> > +   /*
>> > +* If the device is either just a regular device, or an
>> > +* enabled frontend supported by the driver, we add it to our
>> > +* component list.
>> > +*/
>> > +   if (!sun4i_drv_node_is_frontend(node) ||
>> > +   (sun4i_drv_node_is_supported_frontend(node) &&
>> > +of_device_is_available(node))) {
>>
>> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
>> sun4i_drv_node_is_frontend, so of_device_is_available should always
>> be true at this point.
>
> That's not really the condition though :)
>
> It's if the device is *not* a frontend or if it is a supported
> frontend that is available, add it to the endpoints list.

Right. I got confused by the inverted logic. Sorry.

>
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
>> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
>> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
>> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
>> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
>>
>> Seems that you also need to set the "ALPHA_EN" bit for ARGB.
>
> I have not seen that bit documented anywhere. Where is it coming from?

The A31's user manual. I just checked all the datasheets, and only
the A31 and the A80 have this bit (bit 7) defined. It says if the
bit is cleared, alpha is replaced with 0xff. I assume it works either
way on the A33, or you wouldn't be suprised. Leave it out for now then.
(Or maybe a TODO note now that we know about it.)

ChenYu

>
>> > +   frontend->reset = devm_reset_control_get(dev, NULL);
>> > +   if (IS_ERR(frontend->reset)) {
>> > +   dev_err(dev, "Couldn't get our reset line\n");
>> > +   return PTR_ERR(frontend->reset);
>> > +   }
>> > +   reset_control_reset(frontend->reset);
>>
>> reset_control_reset leaves the reset control deasserted. At this
>> point the clock might not be running, which might mean the internal
>> state is not completely wiped out. (Though this really depends on
>> the design of the internal logic.)
>>
>> Maybe just assert it? It gets deasserted in the runtime PM callback
>> later. And just to be safe, I would move it close to the end of the
>> probe path, past all possible errors, so the hardware doesn't get
>> touched until everything is ready. Or don't touch it anywhere in
>> the probe path, and have the runtime PM resume function do a reset.
>
> That seems like the best solution yes.
>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH 1/3] pinctrl: sx150x: unregister the pinctrl on release

[Linus Walleij]

On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin  wrote:

> There is no matching call to pinctrl_unregister, so switch to the
> managed devm_pinctrl_register to clean up properly when done.
>
> Fixes: 9e80f9064e73 ("pinctrl: Add SX150X GPIO Extender Pinctrl Driver")
> Signed-off-by: Peter Rosin 

Patch applied.

Yours,
Linus Walleij

Re: [PATCH 1/3] pinctrl: sx150x: unregister the pinctrl on release

[Linus Walleij]

On Wed, Jan 17, 2018 at 2:34 PM, Peter Rosin  wrote:

> There is no matching call to pinctrl_unregister, so switch to the
> managed devm_pinctrl_register to clean up properly when done.
>
> Fixes: 9e80f9064e73 ("pinctrl: Add SX150X GPIO Extender Pinctrl Driver")
> Signed-off-by: Peter Rosin 

Patch applied.

Yours,
Linus Walleij

[GIT PULL] GPIO fix for v4.15

[Linus Walleij]

Hi Linus,

this is the (hopefully) last GPIO fix for v4.15.

Again the especially endowed screwer-upper who has
been open coding bit fiddling is yours truly.

Please pull it in!

Yours,
Linus Walleij

The following changes since commit a8750ddca918032d6349adbf9a4b6555e7db20da:

  Linux 4.15-rc8 (2018-01-14 15:32:30 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
tags/gpio-v4.15-5

for you to fetch changes up to 07c7b6a52503ac13ae357a8b3ef3456590a64b65:

  gpio: mmio: Also read bits that are zero (2018-01-16 23:42:36 +0100)


GPIO fix for the v4.15 series, this fixes the bit fiddling in the
MMIO GPIO driver.


Linus Walleij (1):
  gpio: mmio: Also read bits that are zero

 drivers/gpio/gpio-mmio.c | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

[GIT PULL] GPIO fix for v4.15

[Linus Walleij]

Hi Linus,

this is the (hopefully) last GPIO fix for v4.15.

Again the especially endowed screwer-upper who has
been open coding bit fiddling is yours truly.

Please pull it in!

Yours,
Linus Walleij

The following changes since commit a8750ddca918032d6349adbf9a4b6555e7db20da:

  Linux 4.15-rc8 (2018-01-14 15:32:30 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git
tags/gpio-v4.15-5

for you to fetch changes up to 07c7b6a52503ac13ae357a8b3ef3456590a64b65:

  gpio: mmio: Also read bits that are zero (2018-01-16 23:42:36 +0100)


GPIO fix for the v4.15 series, this fixes the bit fiddling in the
MMIO GPIO driver.


Linus Walleij (1):
  gpio: mmio: Also read bits that are zero

 drivers/gpio/gpio-mmio.c | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

Re: [RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

On 2018/01/18 15:50, Benjamin Poirier wrote:
> It was reported that emulated e1000e devices in vmware esxi 6.5 Build
> 7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
> overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
> e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
> on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
> icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.
 (_INT_ASSERTED | _LSC)

> 
> Some experimentation showed that this flaw in vmware e1000e emulation can
> be worked around by not setting Other in EIAC. This is how it was before
> 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).
> 
> Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
> Signed-off-by: Benjamin Poirier 

Re: [RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

On 2018/01/18 15:50, Benjamin Poirier wrote:
> It was reported that emulated e1000e devices in vmware esxi 6.5 Build
> 7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
> overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
> e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
> on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
> icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.
 (_INT_ASSERTED | _LSC)

> 
> Some experimentation showed that this flaw in vmware e1000e emulation can
> be worked around by not setting Other in EIAC. This is how it was before
> 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).
> 
> Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
> Signed-off-by: Benjamin Poirier 

Re: [RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

On 2018/01/18 15:50, Benjamin Poirier wrote:
> It was reported that emulated e1000e devices in vmware esxi 6.5 Build
> 7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
> overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
> e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
> on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
> icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.
> 
> Some experimentation showed that this flaw in vmware e1000e emulation can
> be worked around by not setting Other in EIAC. This is how it was before
> 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).

vmware folks, please comment.

Re: [RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

On 2018/01/18 15:50, Benjamin Poirier wrote:
> It was reported that emulated e1000e devices in vmware esxi 6.5 Build
> 7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
> overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
> e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
> on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
> icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.
> 
> Some experimentation showed that this flaw in vmware e1000e emulation can
> be worked around by not setting Other in EIAC. This is how it was before
> 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).

vmware folks, please comment.

Re: AIO operation and CMSG

[Stephan Mueller]

Am Mittwoch, 17. Januar 2018, 20:22:13 CET schrieb Christoph Hellwig:

Hi Christoph,

> On Sun, Jan 14, 2018 at 09:01:00AM +0100, Stephan Müller wrote:
> > The syscall io_submit sends data to the kernel and invokes the respective
> > handler function in the kernel such as the recvmsg handler. What I am
> > wondering is whether there is a way to send CMSG data along with the
> > io_submit syscall? If not, is CMSG handling with the AIO syscalls
> > possible at all?
> Not as-is, but it could be easily added by repurposing unused fields
> in the iocb.  The big question is if it should be done to the existing
> IOCB_CMD_PREADV/IOCB_CMD_PWRITEV types, or if new SENDMSG/RECVMSG ones
> should be added instead.

Thank you for the clarification. I think for the moment we have found another 
solution that we are discussing at linux-crypto. Therefore, I would currently 
not see the need for an additional support of CMSG in AIO.

Ciao
Stephan

Re: AIO operation and CMSG

[Stephan Mueller]

Am Mittwoch, 17. Januar 2018, 20:22:13 CET schrieb Christoph Hellwig:

Hi Christoph,

> On Sun, Jan 14, 2018 at 09:01:00AM +0100, Stephan Müller wrote:
> > The syscall io_submit sends data to the kernel and invokes the respective
> > handler function in the kernel such as the recvmsg handler. What I am
> > wondering is whether there is a way to send CMSG data along with the
> > io_submit syscall? If not, is CMSG handling with the AIO syscalls
> > possible at all?
> Not as-is, but it could be easily added by repurposing unused fields
> in the iocb.  The big question is if it should be done to the existing
> IOCB_CMD_PREADV/IOCB_CMD_PWRITEV types, or if new SENDMSG/RECVMSG ones
> should be added instead.

Thank you for the clarification. I think for the moment we have found another 
solution that we are discussing at linux-crypto. Therefore, I would currently 
not see the need for an additional support of CMSG in AIO.

Ciao
Stephan

Re: [PATCH v4 4/8] PCI: brcmstb: Add dma-range mapping for inbound traffic

[Christoph Hellwig]

On Wed, Jan 17, 2018 at 08:15:33PM -0600, Rob Herring wrote:
> > (a) overriding/redefining the dma_to_phys() and phys_to_dma() calls
> > that are used by the dma_ops routines.  This is the approach of
> >
> > arch/mips/cavium-octeon/dma-octeon.c
> 
> MIPS is rarely an example to follow. :)

But in this case it actually is the example to follow as told previously.

NAK again for these chained dma ops that only create problems.

Re: [PATCH v4 4/8] PCI: brcmstb: Add dma-range mapping for inbound traffic

[Christoph Hellwig]

On Wed, Jan 17, 2018 at 08:15:33PM -0600, Rob Herring wrote:
> > (a) overriding/redefining the dma_to_phys() and phys_to_dma() calls
> > that are used by the dma_ops routines.  This is the approach of
> >
> > arch/mips/cavium-octeon/dma-octeon.c
> 
> MIPS is rarely an example to follow. :)

But in this case it actually is the example to follow as told previously.

NAK again for these chained dma ops that only create problems.

Re: [PATCH V4 1/2] nvme: add NVME_CTRL_RESET_PREPARE state

[jianchao.wang]

Hi James

Thanks for you detailed, kindly response and directive.
That's really appreciated.

On 01/18/2018 02:24 PM, James Smart wrote:
>> So in the patch, RESETTING in nvme-fc/rdma is changed to RESET_PREPARE. Then 
>> we get:
>> nvme-fc/rdma RESET_PREPARE -> RECONNECTING -> LIVE
>> nvme-pci RESET_PREPARE -> RESETTING    -> LIVE
> 
> Right - so another way to look at this is you renamed RESETTING to 
> RESET_PREPARE and added a new RESETTING state in the nvme-pci when in 
> reinits.  Why not simply have the nvme-pci transport transition to 
> RECONNECTING like the other transports. No new states, semantics are all the 
> same.
> 
> Here's what the responsibility of the states are:
> RESETTING:
> -quiescing the blk-mq queues os errors don't bubble back to callees and new 
> io is suspended
> -terminating the link-side association: resets the controller via register 
> access/SET_PROPERTY, deletes connections/queues, cleans out active io and 
> lets them queue for resending, etc.
> -end result is nvme block device is present, but idle, and no active 
> controller on the link side  (link being a transport specific link such as 
> pci, or ethernet/ib for rdma or fc link).
> 
> RECONNECTING:
> - "establish an association at the transport" on PCI this is immediate - you 
> can either talk to the pci function or you can't. With rdma/fc we send 
> transport traffic to create an admin q connection.
> - if association succeeded: the controller is init'd via register 
> accesses/fabric GET/SET_PROPERTIES and admin-queue command, io 
> connections/queues created, blk-mq queues unquiesced, and finally transition 
> to NVME_CTRL_LIVE.
> - if association failed: check controller timeout., if not exceeded, schedule 
> timer and retry later.  With pci, this could cover the temporary loss of the 
> pci function access (a hot plug event) while keeping the nvme blk device live 
> in the system.
> 
> It matches what you are describing but using what we already have in place - 
> with the only difference being the addition of your "boundary" by adding the 
> RECONNECTING state to nvme-pci.

Yes, absolutely. Thanks for your detailed summary here. 
Introducing RECONNECTING into nvme-pci is indeed a clearer way to fix the issue.

> 
> 
>>
 I don't believe RESETTING and RECONNECTING are that similar - unless - you 
 are looking at that "reinit" part that we left grandfathered into PCI.
>> Yes. I have got the point. Thanks for your directive.
>>
>> Both nvme-pc and nvme-fc/rdma have "shutdown" part to tear down 
>> queues/connects, quiesce queues, cancel outstanding requests...
>> We could call this part as "shutdowning" as well as the scheduling gap.
>> Because the difference of initializing between the nvme-pci and 
>> nvme-fc/rdma,  we could call "reiniting" for nvme-pci and
>> call "reconnecting" for nvme-fc/rdma
> 
> I don't think we need different states for the two. The actions taken are 
> really very similar. How they do the actions varies slightly, but not what 
> they are trying to do.   Thus, I'd prefer we keep the existing RECONNECTING 
> state and use it in nvme-pci as well. I'm open to renaming it if there's 
> something about the name that is not agreeable.

Yes. And I will use RECONNECTING in next version to fix this issue. If better 
name, could introduce in other patchset. :)


Many thanks again.
Jianchao

Re: [PATCH V4 1/2] nvme: add NVME_CTRL_RESET_PREPARE state

[jianchao.wang]

Hi James

Thanks for you detailed, kindly response and directive.
That's really appreciated.

On 01/18/2018 02:24 PM, James Smart wrote:
>> So in the patch, RESETTING in nvme-fc/rdma is changed to RESET_PREPARE. Then 
>> we get:
>> nvme-fc/rdma RESET_PREPARE -> RECONNECTING -> LIVE
>> nvme-pci RESET_PREPARE -> RESETTING    -> LIVE
> 
> Right - so another way to look at this is you renamed RESETTING to 
> RESET_PREPARE and added a new RESETTING state in the nvme-pci when in 
> reinits.  Why not simply have the nvme-pci transport transition to 
> RECONNECTING like the other transports. No new states, semantics are all the 
> same.
> 
> Here's what the responsibility of the states are:
> RESETTING:
> -quiescing the blk-mq queues os errors don't bubble back to callees and new 
> io is suspended
> -terminating the link-side association: resets the controller via register 
> access/SET_PROPERTY, deletes connections/queues, cleans out active io and 
> lets them queue for resending, etc.
> -end result is nvme block device is present, but idle, and no active 
> controller on the link side  (link being a transport specific link such as 
> pci, or ethernet/ib for rdma or fc link).
> 
> RECONNECTING:
> - "establish an association at the transport" on PCI this is immediate - you 
> can either talk to the pci function or you can't. With rdma/fc we send 
> transport traffic to create an admin q connection.
> - if association succeeded: the controller is init'd via register 
> accesses/fabric GET/SET_PROPERTIES and admin-queue command, io 
> connections/queues created, blk-mq queues unquiesced, and finally transition 
> to NVME_CTRL_LIVE.
> - if association failed: check controller timeout., if not exceeded, schedule 
> timer and retry later.  With pci, this could cover the temporary loss of the 
> pci function access (a hot plug event) while keeping the nvme blk device live 
> in the system.
> 
> It matches what you are describing but using what we already have in place - 
> with the only difference being the addition of your "boundary" by adding the 
> RECONNECTING state to nvme-pci.

Yes, absolutely. Thanks for your detailed summary here. 
Introducing RECONNECTING into nvme-pci is indeed a clearer way to fix the issue.

> 
> 
>>
 I don't believe RESETTING and RECONNECTING are that similar - unless - you 
 are looking at that "reinit" part that we left grandfathered into PCI.
>> Yes. I have got the point. Thanks for your directive.
>>
>> Both nvme-pc and nvme-fc/rdma have "shutdown" part to tear down 
>> queues/connects, quiesce queues, cancel outstanding requests...
>> We could call this part as "shutdowning" as well as the scheduling gap.
>> Because the difference of initializing between the nvme-pci and 
>> nvme-fc/rdma,  we could call "reiniting" for nvme-pci and
>> call "reconnecting" for nvme-fc/rdma
> 
> I don't think we need different states for the two. The actions taken are 
> really very similar. How they do the actions varies slightly, but not what 
> they are trying to do.   Thus, I'd prefer we keep the existing RECONNECTING 
> state and use it in nvme-pci as well. I'm open to renaming it if there's 
> something about the name that is not agreeable.

Yes. And I will use RECONNECTING in next version to fix this issue. If better 
name, could introduce in other patchset. :)


Many thanks again.
Jianchao

Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled

[Tung Vuong Nguyen]

On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
 wrote:
>
> Hi, Sorry about the delay
>
>
> On 04.01.2018 07:17, Thang Q. Nguyen wrote:
>>
>> Hi,
>>
>> On Sat, Dec 16, 2017 at 10:45 AM, Thang Q. Nguyen  wrote:
>>>
>>> From: Tung Nguyen 
>>>
>>> Currently, hcd->shared_hcd always creates and registers to the usb-core.
>>> If, for some reasons, USB3 downstream port is disabled, no roothub port for
>>> USB3.0 is found. This causes kernel to display an error:
>>> hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19)
>>> This patch checks, creates and registers shared_hcd if USB3.0 downstream
>>> port is available.
>>>
>>> Signed-off-by: Tung Nguyen 
>>> Signed-off-by: Thang Q. Nguyen 
>>> ---
>>>   drivers/usb/host/xhci-mem.c  |  2 +-
>>>   drivers/usb/host/xhci-plat.c | 26 +++--
>>>   drivers/usb/host/xhci.c  | 54 
>>> 
>>>   3 files changed, 54 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>>> index 554a8a5..157d1e7 100644
>>> --- a/drivers/usb/host/xhci-mem.c
>>> +++ b/drivers/usb/host/xhci-mem.c
>>> @@ -1067,7 +1067,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd 
>>> *xhci,
>>>  struct usb_device *top_dev;
>>>  struct usb_hcd *hcd;
>>>
>>> -   if (udev->speed >= USB_SPEED_SUPER)
>>> +   if (udev->speed >= USB_SPEED_SUPER && xhci->shared_hcd)
>>>  hcd = xhci->shared_hcd;
>>>  else
>>>  hcd = xhci->main_hcd;
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 6f03830..e812e3d 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -253,12 +253,6 @@ static int xhci_plat_probe(struct platform_device 
>>> *pdev)
>>>
>>>  xhci->clk = clk;
>>>  xhci->main_hcd = hcd;
>>> -   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
>>> -   dev_name(>dev), hcd);
>>> -   if (!xhci->shared_hcd) {
>>> -   ret = -ENOMEM;
>>> -   goto disable_clk;
>>> -   }
>>>
>>>  if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
>>>  xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>> @@ -290,12 +284,20 @@ static int xhci_plat_probe(struct platform_device 
>>> *pdev)
>>>  if (ret)
>>>  goto disable_usb_phy;
>>>
>>> -   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> -   xhci->shared_hcd->can_do_streams = 1;
>>> -
>>> -   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> -   if (ret)
>>> -   goto dealloc_usb2_hcd;
>>> +   if (xhci->num_usb3_ports > 0) {
>>> +   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, 
>>> >dev,
>>> +   dev_name(>dev), hcd);
>>> +   if (!xhci->shared_hcd) {
>>> +   ret = -ENOMEM;
>>> +   goto disable_clk;
>>> +   }
>>> +   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> +   xhci->shared_hcd->can_do_streams = 1;
>>> +
>>> +   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> +   if (ret)
>>> +   goto dealloc_usb2_hcd;
>>> +   }
>>>
>>>  device_enable_async_suspend(>dev);
>>>  pm_runtime_put_noidle(>dev);
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 05104bd..4824bf6 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -417,12 +417,14 @@ static void compliance_mode_recovery(struct 
>>> timer_list *t)
>>>  i + 1);
>>>  xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
>>>  "Attempting compliance mode 
>>> recovery");
>>> -   hcd = xhci->shared_hcd;
>>> +   if (xhci->shared_hcd) {
>>> +   hcd = xhci->shared_hcd;
>>>
>>> -   if (hcd->state == HC_STATE_SUSPENDED)
>>> -   usb_hcd_resume_root_hub(hcd);
>>> +   if (hcd->state == HC_STATE_SUSPENDED)
>>> +   usb_hcd_resume_root_hub(hcd);
>>>
>>> -   usb_hcd_poll_rh_status(hcd);
>>> +   usb_hcd_poll_rh_status(hcd);
>>> +   }
>>>  }
>>>  }
>>>
>>> @@ -611,6 +613,18 @@ int xhci_run(struct usb_hcd *hcd)
>>>  if (ret)
>>>  xhci_free_command(xhci, command);
>>>  }
>>> +   /*
>>> +* Execute xhci_start() in case xhci->shared_hcd is not registered.
>>> +* If the xhci->shared_hcd doesn't exist, no one triggers to start
>>> +* the xhci 

Re: [PATCH v1 1/1] usb: xhci: do not create and register shared_hcd when USB3.0 is disabled

[Tung Vuong Nguyen]

On Tue, Jan 16, 2018 at 9:50 PM, Mathias Nyman
 wrote:
>
> Hi, Sorry about the delay
>
>
> On 04.01.2018 07:17, Thang Q. Nguyen wrote:
>>
>> Hi,
>>
>> On Sat, Dec 16, 2017 at 10:45 AM, Thang Q. Nguyen  wrote:
>>>
>>> From: Tung Nguyen 
>>>
>>> Currently, hcd->shared_hcd always creates and registers to the usb-core.
>>> If, for some reasons, USB3 downstream port is disabled, no roothub port for
>>> USB3.0 is found. This causes kernel to display an error:
>>> hub 2-0:1.0: config failed, hub doesn't have any ports! (err -19)
>>> This patch checks, creates and registers shared_hcd if USB3.0 downstream
>>> port is available.
>>>
>>> Signed-off-by: Tung Nguyen 
>>> Signed-off-by: Thang Q. Nguyen 
>>> ---
>>>   drivers/usb/host/xhci-mem.c  |  2 +-
>>>   drivers/usb/host/xhci-plat.c | 26 +++--
>>>   drivers/usb/host/xhci.c  | 54 
>>> 
>>>   3 files changed, 54 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>>> index 554a8a5..157d1e7 100644
>>> --- a/drivers/usb/host/xhci-mem.c
>>> +++ b/drivers/usb/host/xhci-mem.c
>>> @@ -1067,7 +1067,7 @@ static u32 xhci_find_real_port_number(struct xhci_hcd 
>>> *xhci,
>>>  struct usb_device *top_dev;
>>>  struct usb_hcd *hcd;
>>>
>>> -   if (udev->speed >= USB_SPEED_SUPER)
>>> +   if (udev->speed >= USB_SPEED_SUPER && xhci->shared_hcd)
>>>  hcd = xhci->shared_hcd;
>>>  else
>>>  hcd = xhci->main_hcd;
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 6f03830..e812e3d 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -253,12 +253,6 @@ static int xhci_plat_probe(struct platform_device 
>>> *pdev)
>>>
>>>  xhci->clk = clk;
>>>  xhci->main_hcd = hcd;
>>> -   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, >dev,
>>> -   dev_name(>dev), hcd);
>>> -   if (!xhci->shared_hcd) {
>>> -   ret = -ENOMEM;
>>> -   goto disable_clk;
>>> -   }
>>>
>>>  if (device_property_read_bool(sysdev, "usb2-lpm-disable"))
>>>  xhci->quirks |= XHCI_HW_LPM_DISABLE;
>>> @@ -290,12 +284,20 @@ static int xhci_plat_probe(struct platform_device 
>>> *pdev)
>>>  if (ret)
>>>  goto disable_usb_phy;
>>>
>>> -   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> -   xhci->shared_hcd->can_do_streams = 1;
>>> -
>>> -   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> -   if (ret)
>>> -   goto dealloc_usb2_hcd;
>>> +   if (xhci->num_usb3_ports > 0) {
>>> +   xhci->shared_hcd = __usb_create_hcd(driver, sysdev, 
>>> >dev,
>>> +   dev_name(>dev), hcd);
>>> +   if (!xhci->shared_hcd) {
>>> +   ret = -ENOMEM;
>>> +   goto disable_clk;
>>> +   }
>>> +   if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
>>> +   xhci->shared_hcd->can_do_streams = 1;
>>> +
>>> +   ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
>>> +   if (ret)
>>> +   goto dealloc_usb2_hcd;
>>> +   }
>>>
>>>  device_enable_async_suspend(>dev);
>>>  pm_runtime_put_noidle(>dev);
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 05104bd..4824bf6 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -417,12 +417,14 @@ static void compliance_mode_recovery(struct 
>>> timer_list *t)
>>>  i + 1);
>>>  xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
>>>  "Attempting compliance mode 
>>> recovery");
>>> -   hcd = xhci->shared_hcd;
>>> +   if (xhci->shared_hcd) {
>>> +   hcd = xhci->shared_hcd;
>>>
>>> -   if (hcd->state == HC_STATE_SUSPENDED)
>>> -   usb_hcd_resume_root_hub(hcd);
>>> +   if (hcd->state == HC_STATE_SUSPENDED)
>>> +   usb_hcd_resume_root_hub(hcd);
>>>
>>> -   usb_hcd_poll_rh_status(hcd);
>>> +   usb_hcd_poll_rh_status(hcd);
>>> +   }
>>>  }
>>>  }
>>>
>>> @@ -611,6 +613,18 @@ int xhci_run(struct usb_hcd *hcd)
>>>  if (ret)
>>>  xhci_free_command(xhci, command);
>>>  }
>>> +   /*
>>> +* Execute xhci_start() in case xhci->shared_hcd is not registered.
>>> +* If the xhci->shared_hcd doesn't exist, no one triggers to start
>>> +* the xhci which should be done before exitting run function
>>> +*/
>>> +   if (!xhci->shared_hcd) {
>>> 

Re: [PATCH v3 09/13] drm/sun4i: backend: Wire in the frontend

[Maxime Ripard]

Hi,

On Wed, Jan 17, 2018 at 09:46:13PM +0800, Chen-Yu Tsai wrote:
> > +   for_each_available_child_of_node(port, ep) {
> > +   remote = of_graph_get_remote_port_parent(ep);
> > +   if (!remote)
> > +   continue;
> > +
> > +   /* does this node match any registered engines? */
> > +   list_for_each_entry(frontend, >frontend_list, list) {
> > +   if (remote == frontend->node) {
> > +   of_node_put(remote);
> > +   of_node_put(port);
> > +   return frontend;
> > +   }
> > +   }
> 
> I assume that since you are only enabling support on the A33, you are
> not considering dual display pipeline cases. We have cross connections
> in the of graph for those situations, i.e. frontend0 can output to both
> backend0 and backend1, as described in the following link:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/arch/arm/boot/dts/sun6i-a31.dtsi#L1177
> 
> So with your code, both backends would end up using frontend0, and
> if both display pipelines are active, one would potentially step on
> the other.
> 
> Instead, have the frontend store its ID in its data structure, and
> have the backend look through the list of frontends to find a match
> for the backend's ID. This is what we already do for engine <-> TCON:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/drivers/gpu/drm/sun4i/sun4i_tcon.c#L583
> 
> Or just leave a TODO note and I can do it later. :)

We discussed it on IRC, and yes, I don't have a test setup right now
with two pipelines being used, and there's a few changes that need to
be made for it to operate properly on the A10/A20. That's why I only
added the A33 in the supported list at the moment.

I'll add a TODO note

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v3 09/13] drm/sun4i: backend: Wire in the frontend

[Maxime Ripard]

Hi,

On Wed, Jan 17, 2018 at 09:46:13PM +0800, Chen-Yu Tsai wrote:
> > +   for_each_available_child_of_node(port, ep) {
> > +   remote = of_graph_get_remote_port_parent(ep);
> > +   if (!remote)
> > +   continue;
> > +
> > +   /* does this node match any registered engines? */
> > +   list_for_each_entry(frontend, >frontend_list, list) {
> > +   if (remote == frontend->node) {
> > +   of_node_put(remote);
> > +   of_node_put(port);
> > +   return frontend;
> > +   }
> > +   }
> 
> I assume that since you are only enabling support on the A33, you are
> not considering dual display pipeline cases. We have cross connections
> in the of graph for those situations, i.e. frontend0 can output to both
> backend0 and backend1, as described in the following link:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/arch/arm/boot/dts/sun6i-a31.dtsi#L1177
> 
> So with your code, both backends would end up using frontend0, and
> if both display pipelines are active, one would potentially step on
> the other.
> 
> Instead, have the frontend store its ID in its data structure, and
> have the backend look through the list of frontends to find a match
> for the backend's ID. This is what we already do for engine <-> TCON:
> 
> http://elixir.free-electrons.com/linux/v4.15-rc8/source/drivers/gpu/drm/sun4i/sun4i_tcon.c#L583
> 
> Or just leave a TODO note and I can do it later. :)

We discussed it on IRC, and yes, I don't have a test setup right now
with two pipelines being used, and there's a few changes that need to
be made for it to operate properly on the A10/A20. That's why I only
added the A33 in the supported list at the moment.

I'll add a TODO note

> Otherwise,
> 
> Reviewed-by: Chen-Yu Tsai 

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

[Maxime Ripard]

Hi,

On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
> > if (sun4i_drv_node_is_connector(node))
> > return 0;
> >
> > -   if (!sun4i_drv_node_is_frontend(node)) {
> > +   /*
> > +* If the device is either just a regular device, or an
> > +* enabled frontend supported by the driver, we add it to our
> > +* component list.
> > +*/
> > +   if (!sun4i_drv_node_is_frontend(node) ||
> > +   (sun4i_drv_node_is_supported_frontend(node) &&
> > +of_device_is_available(node))) {
> 
> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
> sun4i_drv_node_is_frontend, so of_device_is_available should always
> be true at this point.

That's not really the condition though :)

It's if the device is *not* a frontend or if it is a supported
frontend that is available, add it to the endpoints list.

> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
> 
> Seems that you also need to set the "ALPHA_EN" bit for ARGB.

I have not seen that bit documented anywhere. Where is it coming from?

> > +   frontend->reset = devm_reset_control_get(dev, NULL);
> > +   if (IS_ERR(frontend->reset)) {
> > +   dev_err(dev, "Couldn't get our reset line\n");
> > +   return PTR_ERR(frontend->reset);
> > +   }
> > +   reset_control_reset(frontend->reset);
> 
> reset_control_reset leaves the reset control deasserted. At this
> point the clock might not be running, which might mean the internal
> state is not completely wiped out. (Though this really depends on
> the design of the internal logic.)
> 
> Maybe just assert it? It gets deasserted in the runtime PM callback
> later. And just to be safe, I would move it close to the end of the
> probe path, past all possible errors, so the hardware doesn't get
> touched until everything is ready. Or don't touch it anywhere in
> the probe path, and have the runtime PM resume function do a reset.

That seems like the best solution yes.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v3 08/13] drm/sun4i: Add a driver for the display frontend

[Maxime Ripard]

Hi,

On Wed, Jan 17, 2018 at 09:43:31PM +0800, Chen-Yu Tsai wrote:
> > if (sun4i_drv_node_is_connector(node))
> > return 0;
> >
> > -   if (!sun4i_drv_node_is_frontend(node)) {
> > +   /*
> > +* If the device is either just a regular device, or an
> > +* enabled frontend supported by the driver, we add it to our
> > +* component list.
> > +*/
> > +   if (!sun4i_drv_node_is_frontend(node) ||
> > +   (sun4i_drv_node_is_supported_frontend(node) &&
> > +of_device_is_available(node))) {
> 
> Nit: sun4i_drv_node_is_supported_frontend should be a subset of
> sun4i_drv_node_is_frontend, so of_device_is_available should always
> be true at this point.

That's not really the condition though :)

It's if the device is *not* a frontend or if it is a supported
frontend that is available, add it to the endpoints list.

> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_INPUT_FMT_REG,
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_MOD(1) |
> > +SUN4I_FRONTEND_INPUT_FMT_DATA_FMT(in_fmt_val) |
> > +SUN4I_FRONTEND_INPUT_FMT_PS(1));
> > +   regmap_write(frontend->regs, SUN4I_FRONTEND_OUTPUT_FMT_REG,
> > +SUN4I_FRONTEND_OUTPUT_FMT_DATA_FMT(out_fmt_val));
> 
> Seems that you also need to set the "ALPHA_EN" bit for ARGB.

I have not seen that bit documented anywhere. Where is it coming from?

> > +   frontend->reset = devm_reset_control_get(dev, NULL);
> > +   if (IS_ERR(frontend->reset)) {
> > +   dev_err(dev, "Couldn't get our reset line\n");
> > +   return PTR_ERR(frontend->reset);
> > +   }
> > +   reset_control_reset(frontend->reset);
> 
> reset_control_reset leaves the reset control deasserted. At this
> point the clock might not be running, which might mean the internal
> state is not completely wiped out. (Though this really depends on
> the design of the internal logic.)
> 
> Maybe just assert it? It gets deasserted in the runtime PM callback
> later. And just to be safe, I would move it close to the end of the
> probe path, past all possible errors, so the hardware doesn't get
> touched until everything is ready. Or don't touch it anywhere in
> the probe path, and have the runtime PM resume function do a reset.

That seems like the best solution yes.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

linux-next: Tree for Jan 18

[Stephen Rothwell]

Hi all,

Changes since 20180117:

The powerpc tree gained a conflict against the powerpc-fixes tree.

The f2fs tree gained a build failure due to an interaction with the
btrfs tree for which I reverted a commit.

The net-next tree gained a conflict against the net tree.

Non-merge commits (relative to Linus' tree): 9595
 9650 files changed, 397215 insertions(+), 259560 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a
multi_v7_defconfig for arm and a native build of tools/perf. After
the final fixups (if any), I do an x86_64 modules_install followed by
builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit),
ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc
and sparc64 defconfig. And finally, a simple boot test of the powerpc
pseries_le_defconfig kernel in qemu (with and without kvm enabled).

Below is a summary of the state of the merge.

I am currently merging 256 trees (counting Linus' and 44 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (88dc7fca1800 Merge branch 'x86-pti-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip)
Merging fixes/master (820bf5c419e4 Merge tag 'scsi-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging kbuild-current/fixes (36c1681678b5 genksyms: drop *.hash.c from 
.gitignore)
Merging arc-current/for-curr (8bbfbc2df6e9 ARCv2: cache: fix slc_entire_op: 
flush only instead of flush-n-inv)
Merging arm-current/fixes (091f02483df7 ARM: net: bpf: clarify tail_call index)
Merging m68k-current/for-linus (5e387199c17c m68k/defconfig: Update defconfigs 
for v4.14-rc7)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (1b689a95ce74 powerpc/pseries: include 
linux/types.h in asm/hvcall.h)
Merging sparc/master (59585b4be9ae sparc64: repair calling incorrect hweight 
function from stubs)
Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2)
Merging net/master (0b1655143df0 r8152: disable RX aggregation on Dell TB16 
dock)
Merging bpf/master (f37a8cb84cce bpf: reject stores into ctx via st and xadd)
Merging ipsec/master (76a420119181 xfrm: Fix a race in the xdst pcpu cache.)
Merging netfilter/master (889c604fd0b5 netfilter: x_tables: fix int overflow in 
xt_alloc_table_info())
Merging ipvs/master (f7fb77fc1235 netfilter: nft_compat: check extension hook 
mask only if set)
Merging wireless-drivers/master (cc124d5cc8d8 brcmfmac: fix CLM load error for 
legacy chips when user helper is enabled)
Merging mac80211/master (59b179b48ce2 cfg80211: check dev_set_name() return 
value)
Merging rdma-fixes/for-rc (ae59c3f0b6cf RDMA/mlx5: Fix out-of-bound access 
while querying AH)
Merging sound-current/for-linus (b3defb791b26 ALSA: seq: Make ioctls race-free)
Merging pci-current/for-linus (d6c1efecd1e1 x86/PCI: Enable AMD 64-bit window 
on resume)
Merging driver-core.current/driver-core-linus (30a7acd57389 Linux 4.15-rc6)
Merging tty.current/tty-linus (30a7acd57389 Linux 4.15-rc6)
Merging usb.current/usb-linus (a8750ddca918 Linux 4.15-rc8)
Merging usb-gadget-fixes/fixes (b2cd1df66037 Linux 4.15-rc7)
Merging usb-serial-fixes/usb-linus (d14ac576d10f USB: serial: cp210x: add new 
device ID ELV ALC 8xxx)
Merging usb-chipidea-fixes/ci-for-usb-stable (964728f9f407 USB: chipidea: msm: 
fix ulpi-node lookup)
Merging phy/fixes (2b88212c4cc6 phy: rcar-gen3-usb2: select USB_COMMON)
Merging staging.current/staging-linus (a8750ddca918 Linux 4.15-rc8)
Merging char-misc.current/char-misc-linus (a8750ddca918 Linux 4.15-rc8)
Merging input-current/for-linus (4d94e776bd29 Input: ALPS - fix multi-touch 
decoding on SS4 plus touchpads)
Merging crypto-current/master (9a00674213a3 crypto: algapi - fix NULL 
dereference in crypto_remove_spawns()

linux-next: Tree for Jan 18

[Stephen Rothwell]

Hi all,

Changes since 20180117:

The powerpc tree gained a conflict against the powerpc-fixes tree.

The f2fs tree gained a build failure due to an interaction with the
btrfs tree for which I reverted a commit.

The net-next tree gained a conflict against the net tree.

Non-merge commits (relative to Linus' tree): 9595
 9650 files changed, 397215 insertions(+), 259560 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a
multi_v7_defconfig for arm and a native build of tools/perf. After
the final fixups (if any), I do an x86_64 modules_install followed by
builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit),
ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc
and sparc64 defconfig. And finally, a simple boot test of the powerpc
pseries_le_defconfig kernel in qemu (with and without kvm enabled).

Below is a summary of the state of the merge.

I am currently merging 256 trees (counting Linus' and 44 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (88dc7fca1800 Merge branch 'x86-pti-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip)
Merging fixes/master (820bf5c419e4 Merge tag 'scsi-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging kbuild-current/fixes (36c1681678b5 genksyms: drop *.hash.c from 
.gitignore)
Merging arc-current/for-curr (8bbfbc2df6e9 ARCv2: cache: fix slc_entire_op: 
flush only instead of flush-n-inv)
Merging arm-current/fixes (091f02483df7 ARM: net: bpf: clarify tail_call index)
Merging m68k-current/for-linus (5e387199c17c m68k/defconfig: Update defconfigs 
for v4.14-rc7)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (1b689a95ce74 powerpc/pseries: include 
linux/types.h in asm/hvcall.h)
Merging sparc/master (59585b4be9ae sparc64: repair calling incorrect hweight 
function from stubs)
Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2)
Merging net/master (0b1655143df0 r8152: disable RX aggregation on Dell TB16 
dock)
Merging bpf/master (f37a8cb84cce bpf: reject stores into ctx via st and xadd)
Merging ipsec/master (76a420119181 xfrm: Fix a race in the xdst pcpu cache.)
Merging netfilter/master (889c604fd0b5 netfilter: x_tables: fix int overflow in 
xt_alloc_table_info())
Merging ipvs/master (f7fb77fc1235 netfilter: nft_compat: check extension hook 
mask only if set)
Merging wireless-drivers/master (cc124d5cc8d8 brcmfmac: fix CLM load error for 
legacy chips when user helper is enabled)
Merging mac80211/master (59b179b48ce2 cfg80211: check dev_set_name() return 
value)
Merging rdma-fixes/for-rc (ae59c3f0b6cf RDMA/mlx5: Fix out-of-bound access 
while querying AH)
Merging sound-current/for-linus (b3defb791b26 ALSA: seq: Make ioctls race-free)
Merging pci-current/for-linus (d6c1efecd1e1 x86/PCI: Enable AMD 64-bit window 
on resume)
Merging driver-core.current/driver-core-linus (30a7acd57389 Linux 4.15-rc6)
Merging tty.current/tty-linus (30a7acd57389 Linux 4.15-rc6)
Merging usb.current/usb-linus (a8750ddca918 Linux 4.15-rc8)
Merging usb-gadget-fixes/fixes (b2cd1df66037 Linux 4.15-rc7)
Merging usb-serial-fixes/usb-linus (d14ac576d10f USB: serial: cp210x: add new 
device ID ELV ALC 8xxx)
Merging usb-chipidea-fixes/ci-for-usb-stable (964728f9f407 USB: chipidea: msm: 
fix ulpi-node lookup)
Merging phy/fixes (2b88212c4cc6 phy: rcar-gen3-usb2: select USB_COMMON)
Merging staging.current/staging-linus (a8750ddca918 Linux 4.15-rc8)
Merging char-misc.current/char-misc-linus (a8750ddca918 Linux 4.15-rc8)
Merging input-current/for-linus (4d94e776bd29 Input: ALPS - fix multi-touch 
decoding on SS4 plus touchpads)
Merging crypto-current/master (9a00674213a3 crypto: algapi - fix NULL 
dereference in crypto_remove_spawns()

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 07:40:35PM +0100, Andreas Kemnade wrote:
> On Fri, 12 Jan 2018 15:46:47 +0100
> Johan Hovold  wrote:
> 
> > On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > > On Fri, 22 Dec 2017 13:44:27 +0100
> > > Johan Hovold  wrote:
> > > 
> > > [...]  
> > > > I'd suggest reiterating the problem you're trying to solve and
> > > > enumerating the previously discussed potential solutions in order to
> > > > find a proper abstraction level for this (before getting lost in
> > > > implementation details).
> > > >   
> > > The main point here is in short words: Having a device powered on or off
> > > when the uart it is attached to, is used or not used anymore,
> > > so the already available userspace applications do not need to be 
> > > changed.  
> > 
> > So we'd end up with something in-between a kernel driver and a
> > user-space solution. What about devices that need to be (partially)
> > powered also when the port isn't open? A pure user-space solution would
> > be able to handle all variants.
> > 
> Well partly powered devices are at many places, And they hide that problem
> from userspace, just get the open()/get() and close()/put() from there and 
> power the
> device accordingly. 
> 
> So the question still remains why should the kernel hide some things and some
> it should not.
> If it all is in userspace, then there is still something needed in the 
> devicetree
> (if I understand correctly, every information about hardware which cannot be
> auto-probed belongs into device tree) so that the userspace knows what kind of
> device is at that port. So there can be a daemon powering on and off devices.
> But that would break existing applications which just expect that they just 
> need
> to open/close the device. 
> 
> Or you need to have some inotify handler in userspace and attach it there to
> react on close() and open() of that device.
> But this thing needs to have two kind of information:
> 
> 1. the type of chip available to do the right powerup sequence. 
> 
> 2. how the chip is wired up to the cpu. 
> 
> So to avoid having hardware information spread all over the table at least
> these information would need to be in devicetree. But that also all feels
> like a hack and hard to maintain.

Having the device described in the device tree is certainly desirable,
not least for chip identification. And with a GPS framework in the
kernel with a well-defined interface, implementing power management
would be straight forward.

I'm just not convinced that the proposed tty interface is the right
interface for this. User space would still rely on gpsd for the GPS
protocols, and would also ultimately be managing power by killing gpsd
or whatever daemon that would otherwise be holding the port open.

Something like the generic power sequences that has been discussed
elsewhere might be a better fit for this if all you want to do is power
on and off on port open and close (and on suspend/resume). There really
isn't anything GPS-specific in the current proposal (besides the
suggested tty-device name).

But sure, that wouldn't be sufficient to deal with the
unknown-power-state problem with the device in question.

Johan

Re: [Letux-kernel] [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 07:40:35PM +0100, Andreas Kemnade wrote:
> On Fri, 12 Jan 2018 15:46:47 +0100
> Johan Hovold  wrote:
> 
> > On Tue, Jan 09, 2018 at 06:43:47PM +0100, Andreas Kemnade wrote:
> > > On Fri, 22 Dec 2017 13:44:27 +0100
> > > Johan Hovold  wrote:
> > > 
> > > [...]  
> > > > I'd suggest reiterating the problem you're trying to solve and
> > > > enumerating the previously discussed potential solutions in order to
> > > > find a proper abstraction level for this (before getting lost in
> > > > implementation details).
> > > >   
> > > The main point here is in short words: Having a device powered on or off
> > > when the uart it is attached to, is used or not used anymore,
> > > so the already available userspace applications do not need to be 
> > > changed.  
> > 
> > So we'd end up with something in-between a kernel driver and a
> > user-space solution. What about devices that need to be (partially)
> > powered also when the port isn't open? A pure user-space solution would
> > be able to handle all variants.
> > 
> Well partly powered devices are at many places, And they hide that problem
> from userspace, just get the open()/get() and close()/put() from there and 
> power the
> device accordingly. 
> 
> So the question still remains why should the kernel hide some things and some
> it should not.
> If it all is in userspace, then there is still something needed in the 
> devicetree
> (if I understand correctly, every information about hardware which cannot be
> auto-probed belongs into device tree) so that the userspace knows what kind of
> device is at that port. So there can be a daemon powering on and off devices.
> But that would break existing applications which just expect that they just 
> need
> to open/close the device. 
> 
> Or you need to have some inotify handler in userspace and attach it there to
> react on close() and open() of that device.
> But this thing needs to have two kind of information:
> 
> 1. the type of chip available to do the right powerup sequence. 
> 
> 2. how the chip is wired up to the cpu. 
> 
> So to avoid having hardware information spread all over the table at least
> these information would need to be in devicetree. But that also all feels
> like a hack and hard to maintain.

Having the device described in the device tree is certainly desirable,
not least for chip identification. And with a GPS framework in the
kernel with a well-defined interface, implementing power management
would be straight forward.

I'm just not convinced that the proposed tty interface is the right
interface for this. User space would still rely on gpsd for the GPS
protocols, and would also ultimately be managing power by killing gpsd
or whatever daemon that would otherwise be holding the port open.

Something like the generic power sequences that has been discussed
elsewhere might be a better fit for this if all you want to do is power
on and off on port open and close (and on suspend/resume). There really
isn't anything GPS-specific in the current proposal (besides the
suggested tty-device name).

But sure, that wouldn't be sufficient to deal with the
unknown-power-state problem with the device in question.

Johan

[RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

It was reported that emulated e1000e devices in vmware esxi 6.5 Build
7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.

Some experimentation showed that this flaw in vmware e1000e emulation can
be worked around by not setting Other in EIAC. This is how it was before
16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).

Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
Signed-off-by: Benjamin Poirier 
---

Jeff, I'm sending as RFC since it looks like a problem that should be fixed
in vmware. If you'd like to have the workaround in e1000e, I'll submit.

---
 drivers/net/ethernet/intel/e1000e/netdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9f18d39bdc8f..625a4c9a86a4 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1918,6 +1918,8 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
bool enable = true;
 
icr = er32(ICR);
+   ew32(ICR, E1000_ICR_OTHER);
+
if (icr & E1000_ICR_RXO) {
ew32(ICR, E1000_ICR_RXO);
enable = false;
@@ -2040,7 +2042,6 @@ static void e1000_configure_msix(struct e1000_adapter 
*adapter)
   hw->hw_addr + E1000_EITR_82574(vector));
else
writel(1, hw->hw_addr + E1000_EITR_82574(vector));
-   adapter->eiac_mask |= E1000_IMS_OTHER;
 
/* Cause Tx interrupts on every write back */
ivar |= BIT(31);
@@ -2265,7 +2266,7 @@ static void e1000_irq_enable(struct e1000_adapter 
*adapter)
 
if (adapter->msix_entries) {
ew32(EIAC_82574, adapter->eiac_mask & E1000_EIAC_MASK_82574);
-   ew32(IMS, adapter->eiac_mask | E1000_IMS_LSC);
+   ew32(IMS, adapter->eiac_mask | E1000_IMS_OTHER | E1000_IMS_LSC);
} else if (hw->mac.type >= e1000_pch_lpt) {
ew32(IMS, IMS_ENABLE_MASK | E1000_IMS_ECCER);
} else {
-- 
2.15.1

[RFC PATCH] e1000e: Remove Other from EIAC.

[Benjamin Poirier]

It was reported that emulated e1000e devices in vmware esxi 6.5 Build
7526125 do not link up after commit 4aea7a5c5e94 ("e1000e: Avoid receiver
overrun interrupt bursts", v4.15-rc1). Some tracing shows that after
e1000e_trigger_lsc() is called, ICR reads out as 0x0 in e1000_msix_other()
on emulated e1000e devices. In comparison, on real e1000e 82574 hardware,
icr=0x8004 (_INT_ASSERTED | _OTHER) in the same situation.

Some experimentation showed that this flaw in vmware e1000e emulation can
be worked around by not setting Other in EIAC. This is how it was before
16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt", v4.5-rc1).

Fixes: 4aea7a5c5e94 ("e1000e: Avoid receiver overrun interrupt bursts")
Signed-off-by: Benjamin Poirier 
---

Jeff, I'm sending as RFC since it looks like a problem that should be fixed
in vmware. If you'd like to have the workaround in e1000e, I'll submit.

---
 drivers/net/ethernet/intel/e1000e/netdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9f18d39bdc8f..625a4c9a86a4 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1918,6 +1918,8 @@ static irqreturn_t e1000_msix_other(int __always_unused 
irq, void *data)
bool enable = true;
 
icr = er32(ICR);
+   ew32(ICR, E1000_ICR_OTHER);
+
if (icr & E1000_ICR_RXO) {
ew32(ICR, E1000_ICR_RXO);
enable = false;
@@ -2040,7 +2042,6 @@ static void e1000_configure_msix(struct e1000_adapter 
*adapter)
   hw->hw_addr + E1000_EITR_82574(vector));
else
writel(1, hw->hw_addr + E1000_EITR_82574(vector));
-   adapter->eiac_mask |= E1000_IMS_OTHER;
 
/* Cause Tx interrupts on every write back */
ivar |= BIT(31);
@@ -2265,7 +2266,7 @@ static void e1000_irq_enable(struct e1000_adapter 
*adapter)
 
if (adapter->msix_entries) {
ew32(EIAC_82574, adapter->eiac_mask & E1000_EIAC_MASK_82574);
-   ew32(IMS, adapter->eiac_mask | E1000_IMS_LSC);
+   ew32(IMS, adapter->eiac_mask | E1000_IMS_OTHER | E1000_IMS_LSC);
} else if (hw->mac.type >= e1000_pch_lpt) {
ew32(IMS, IMS_ENABLE_MASK | E1000_IMS_ECCER);
} else {
-- 
2.15.1

Re: [PATCH] cfg80211: stop demanding lots of new stuff

[Johannes Berg]

On Wed, 2018-01-17 at 14:55 -0800, Hugh Dickins wrote:
> "make oldconfig" from 4.14 (when CONFIG_CFG80211_CERTIFICATION_ONUS
> is not set) to 4.15-rc, gets into asking lots of new questions, and
> configuring in unwanted stuff: I'm unsure of my Kconfig skills, but
> it looks like CFG80211_REQUIRE_SIGNED_REGDB's "default y" needs to
> be toned down when we don't even have CFG80211_CERTIFICATION_ONUS.

No, this is wrong - we want default configurations to be able to load a
signed regulatory database and validate the signature.

johannes

Re: [PATCH] cfg80211: stop demanding lots of new stuff

[Johannes Berg]

On Wed, 2018-01-17 at 14:55 -0800, Hugh Dickins wrote:
> "make oldconfig" from 4.14 (when CONFIG_CFG80211_CERTIFICATION_ONUS
> is not set) to 4.15-rc, gets into asking lots of new questions, and
> configuring in unwanted stuff: I'm unsure of my Kconfig skills, but
> it looks like CFG80211_REQUIRE_SIGNED_REGDB's "default y" needs to
> be toned down when we don't even have CFG80211_CERTIFICATION_ONUS.

No, this is wrong - we want default configurations to be able to load a
signed regulatory database and validate the signature.

johannes

[PATCH net-next 5/5] net: hns3: add net status led support for fiber port

[Peng Li]

From: Jian Shen 

Check the net status per second, include port speed, total rx/tx packets
and link status. Updating the led status for fiber port.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |   1 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 109 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|   2 +
 3 files changed, 112 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 122f862..3fd10a6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -115,6 +115,7 @@ enum hclge_opcode_type {
HCLGE_OPC_QUERY_LINK_STATUS = 0x0307,
HCLGE_OPC_CONFIG_MAX_FRM_SIZE   = 0x0308,
HCLGE_OPC_CONFIG_SPEED_DUP  = 0x0309,
+   HCLGE_OPC_STATS_MAC_TRAFFIC = 0x0314,
/* MACSEC command */
 
/* PFC/Pause CMD*/
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 039c90b..f3d9332 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -39,6 +39,7 @@ static int hclge_set_mta_filter_mode(struct hclge_dev *hdev,
 static int hclge_set_mtu(struct hnae3_handle *handle, int new_mtu);
 static int hclge_init_vlan_config(struct hclge_dev *hdev);
 static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev);
+static int hclge_update_led_status(struct hclge_dev *hdev);
 
 static struct hnae3_ae_algo ae_algo;
 
@@ -505,6 +506,38 @@ static int hclge_32_bit_update_stats(struct hclge_dev 
*hdev)
return 0;
 }
 
+static int hclge_mac_get_traffic_stats(struct hclge_dev *hdev)
+{
+   struct hclge_mac_stats *mac_stats = >hw_stats.mac_stats;
+   struct hclge_desc desc;
+   __le64 *desc_data;
+   int ret;
+
+   /* for fiber port, need to query the total rx/tx packets statstics,
+* used for data transferring checking.
+*/
+   if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_FIBER)
+   return 0;
+
+   if (test_bit(HCLGE_STATE_STATISTICS_UPDATING, >state))
+   return 0;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_STATS_MAC_TRAFFIC, true);
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "Get MAC total pkt stats fail, ret = %d\n", ret);
+
+   return ret;
+   }
+
+   desc_data = (__le64 *)([0]);
+   mac_stats->mac_tx_total_pkt_num += le64_to_cpu(*desc_data++);
+   mac_stats->mac_rx_total_pkt_num += le64_to_cpu(*desc_data);
+
+   return 0;
+}
+
 static int hclge_mac_update_stats(struct hclge_dev *hdev)
 {
 #define HCLGE_MAC_CMD_NUM 21
@@ -2846,13 +2879,20 @@ static void hclge_service_task(struct work_struct *work)
struct hclge_dev *hdev =
container_of(work, struct hclge_dev, service_task);
 
+   /* The total rx/tx packets statstics are wanted to be updated
+* per second. Both hclge_update_stats_for_all() and
+* hclge_mac_get_traffic_stats() can do it.
+*/
if (hdev->hw_stats.stats_timer >= HCLGE_STATS_TIMER_INTERVAL) {
hclge_update_stats_for_all(hdev);
hdev->hw_stats.stats_timer = 0;
+   } else {
+   hclge_mac_get_traffic_stats(hdev);
}
 
hclge_update_speed_duplex(hdev);
hclge_update_link_status(hdev);
+   hclge_update_led_status(hdev);
hclge_service_complete(hdev);
 }
 
@@ -5965,6 +6005,75 @@ static int hclge_set_led_id(struct hnae3_handle *handle,
return ret;
 }
 
+enum hclge_led_port_speed {
+   HCLGE_SPEED_LED_FOR_1G,
+   HCLGE_SPEED_LED_FOR_10G,
+   HCLGE_SPEED_LED_FOR_25G,
+   HCLGE_SPEED_LED_FOR_40G,
+   HCLGE_SPEED_LED_FOR_50G,
+   HCLGE_SPEED_LED_FOR_100G,
+};
+
+static u8 hclge_led_get_speed_status(u32 speed)
+{
+   u8 speed_led;
+
+   switch (speed) {
+   case HCLGE_MAC_SPEED_1G:
+   speed_led = HCLGE_SPEED_LED_FOR_1G;
+   break;
+   case HCLGE_MAC_SPEED_10G:
+   speed_led = HCLGE_SPEED_LED_FOR_10G;
+   break;
+   case HCLGE_MAC_SPEED_25G:
+   speed_led = HCLGE_SPEED_LED_FOR_25G;
+   break;
+   case HCLGE_MAC_SPEED_40G:
+   speed_led = HCLGE_SPEED_LED_FOR_40G;
+   break;
+   case HCLGE_MAC_SPEED_50G:
+   speed_led = HCLGE_SPEED_LED_FOR_50G;
+   break;
+   case HCLGE_MAC_SPEED_100G:
+   speed_led = HCLGE_SPEED_LED_FOR_100G;
+   break;
+   default:
+   speed_led = HCLGE_LED_NO_CHANGE;
+   }
+
+   return speed_led;
+}
+
+static int hclge_update_led_status(struct 

[PATCH net-next 0/5] add some features to hns3 driver

[Peng Li]

This patchset adds some features to hns3 driver, include the support
for ethtool command -d, -p and support for manager table.

[Patch 1/5] adds support for ethtool command -d, its ops is get_regs.
driver will send command to command queue, and get regs number and
regs value from command queue.
[Patch 2/5] adds manager table initialization for hardware.
[Patch 3/5 - 4/5] add support for ethtool command -p. Phy will control
leds for copper ports. For fiber ports, driver sends command to
command queue, and IMP will write SGPIO regs to control leds.
[Patch 4/5] adds support for net status led for fiber ports. Net status
include  port speed, total rx/tx packets and link status. Driver send
the status to command queue, and IMP will write SGPIO to control leds.

Fuyun Liang (2):
  net: hns3: add support for get_regs
  net: hns3: add manager table initialization for hardware

Jian Shen (3):
  net: hns3: add ethtool -p support for phy device
  net: hns3: add ethtool -p support for fiber port
  net: hns3: add net status led support for fiber port

 drivers/net/ethernet/hisilicon/hns3/hnae3.h|   5 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  35 ++
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  47 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 533 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|  14 +
 5 files changed, 633 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH net-next 5/5] net: hns3: add net status led support for fiber port

[Peng Li]

From: Jian Shen 

Check the net status per second, include port speed, total rx/tx packets
and link status. Updating the led status for fiber port.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |   1 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 109 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|   2 +
 3 files changed, 112 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 122f862..3fd10a6 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -115,6 +115,7 @@ enum hclge_opcode_type {
HCLGE_OPC_QUERY_LINK_STATUS = 0x0307,
HCLGE_OPC_CONFIG_MAX_FRM_SIZE   = 0x0308,
HCLGE_OPC_CONFIG_SPEED_DUP  = 0x0309,
+   HCLGE_OPC_STATS_MAC_TRAFFIC = 0x0314,
/* MACSEC command */
 
/* PFC/Pause CMD*/
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 039c90b..f3d9332 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -39,6 +39,7 @@ static int hclge_set_mta_filter_mode(struct hclge_dev *hdev,
 static int hclge_set_mtu(struct hnae3_handle *handle, int new_mtu);
 static int hclge_init_vlan_config(struct hclge_dev *hdev);
 static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev);
+static int hclge_update_led_status(struct hclge_dev *hdev);
 
 static struct hnae3_ae_algo ae_algo;
 
@@ -505,6 +506,38 @@ static int hclge_32_bit_update_stats(struct hclge_dev 
*hdev)
return 0;
 }
 
+static int hclge_mac_get_traffic_stats(struct hclge_dev *hdev)
+{
+   struct hclge_mac_stats *mac_stats = >hw_stats.mac_stats;
+   struct hclge_desc desc;
+   __le64 *desc_data;
+   int ret;
+
+   /* for fiber port, need to query the total rx/tx packets statstics,
+* used for data transferring checking.
+*/
+   if (hdev->hw.mac.media_type != HNAE3_MEDIA_TYPE_FIBER)
+   return 0;
+
+   if (test_bit(HCLGE_STATE_STATISTICS_UPDATING, >state))
+   return 0;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_STATS_MAC_TRAFFIC, true);
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "Get MAC total pkt stats fail, ret = %d\n", ret);
+
+   return ret;
+   }
+
+   desc_data = (__le64 *)([0]);
+   mac_stats->mac_tx_total_pkt_num += le64_to_cpu(*desc_data++);
+   mac_stats->mac_rx_total_pkt_num += le64_to_cpu(*desc_data);
+
+   return 0;
+}
+
 static int hclge_mac_update_stats(struct hclge_dev *hdev)
 {
 #define HCLGE_MAC_CMD_NUM 21
@@ -2846,13 +2879,20 @@ static void hclge_service_task(struct work_struct *work)
struct hclge_dev *hdev =
container_of(work, struct hclge_dev, service_task);
 
+   /* The total rx/tx packets statstics are wanted to be updated
+* per second. Both hclge_update_stats_for_all() and
+* hclge_mac_get_traffic_stats() can do it.
+*/
if (hdev->hw_stats.stats_timer >= HCLGE_STATS_TIMER_INTERVAL) {
hclge_update_stats_for_all(hdev);
hdev->hw_stats.stats_timer = 0;
+   } else {
+   hclge_mac_get_traffic_stats(hdev);
}
 
hclge_update_speed_duplex(hdev);
hclge_update_link_status(hdev);
+   hclge_update_led_status(hdev);
hclge_service_complete(hdev);
 }
 
@@ -5965,6 +6005,75 @@ static int hclge_set_led_id(struct hnae3_handle *handle,
return ret;
 }
 
+enum hclge_led_port_speed {
+   HCLGE_SPEED_LED_FOR_1G,
+   HCLGE_SPEED_LED_FOR_10G,
+   HCLGE_SPEED_LED_FOR_25G,
+   HCLGE_SPEED_LED_FOR_40G,
+   HCLGE_SPEED_LED_FOR_50G,
+   HCLGE_SPEED_LED_FOR_100G,
+};
+
+static u8 hclge_led_get_speed_status(u32 speed)
+{
+   u8 speed_led;
+
+   switch (speed) {
+   case HCLGE_MAC_SPEED_1G:
+   speed_led = HCLGE_SPEED_LED_FOR_1G;
+   break;
+   case HCLGE_MAC_SPEED_10G:
+   speed_led = HCLGE_SPEED_LED_FOR_10G;
+   break;
+   case HCLGE_MAC_SPEED_25G:
+   speed_led = HCLGE_SPEED_LED_FOR_25G;
+   break;
+   case HCLGE_MAC_SPEED_40G:
+   speed_led = HCLGE_SPEED_LED_FOR_40G;
+   break;
+   case HCLGE_MAC_SPEED_50G:
+   speed_led = HCLGE_SPEED_LED_FOR_50G;
+   break;
+   case HCLGE_MAC_SPEED_100G:
+   speed_led = HCLGE_SPEED_LED_FOR_100G;
+   break;
+   default:
+   speed_led = HCLGE_LED_NO_CHANGE;
+   }
+
+   return speed_led;
+}
+
+static int hclge_update_led_status(struct hclge_dev *hdev)
+{
+   u8 port_speed_status, link_status, 

[PATCH net-next 0/5] add some features to hns3 driver

[Peng Li]

This patchset adds some features to hns3 driver, include the support
for ethtool command -d, -p and support for manager table.

[Patch 1/5] adds support for ethtool command -d, its ops is get_regs.
driver will send command to command queue, and get regs number and
regs value from command queue.
[Patch 2/5] adds manager table initialization for hardware.
[Patch 3/5 - 4/5] add support for ethtool command -p. Phy will control
leds for copper ports. For fiber ports, driver sends command to
command queue, and IMP will write SGPIO regs to control leds.
[Patch 4/5] adds support for net status led for fiber ports. Net status
include  port speed, total rx/tx packets and link status. Driver send
the status to command queue, and IMP will write SGPIO to control leds.

Fuyun Liang (2):
  net: hns3: add support for get_regs
  net: hns3: add manager table initialization for hardware

Jian Shen (3):
  net: hns3: add ethtool -p support for phy device
  net: hns3: add ethtool -p support for fiber port
  net: hns3: add net status led support for fiber port

 drivers/net/ethernet/hisilicon/hns3/hnae3.h|   5 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  35 ++
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  47 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 533 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h|  14 +
 5 files changed, 633 insertions(+), 1 deletion(-)

-- 
2.9.3

[PATCH net-next 1/5] net: hns3: add support for get_regs

[Peng Li]

From: Fuyun Liang 

This patch adds get_regs support for ethtool cmd.

Signed-off-by: Fuyun Liang 
Signed-off-by: Peng Li 
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h|   3 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  23 +++
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |   4 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 176 +
 4 files changed, 205 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h 
b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 634e932..d104ce5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -356,7 +356,8 @@ struct hnae3_ae_ops {
u32 stringset, u8 *data);
int (*get_sset_count)(struct hnae3_handle *handle, int stringset);
 
-   void (*get_regs)(struct hnae3_handle *handle, void *data);
+   void (*get_regs)(struct hnae3_handle *handle, u32 *version,
+void *data);
int (*get_regs_len)(struct hnae3_handle *handle);
 
u32 (*get_rss_key_size)(struct hnae3_handle *handle);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 358f780..1c8b293 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -1063,6 +1063,27 @@ static int hns3_set_coalesce(struct net_device *netdev,
return 0;
 }
 
+static int hns3_get_regs_len(struct net_device *netdev)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo->ops->get_regs_len)
+   return -EOPNOTSUPP;
+
+   return h->ae_algo->ops->get_regs_len(h);
+}
+
+static void hns3_get_regs(struct net_device *netdev,
+ struct ethtool_regs *cmd, void *data)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo->ops->get_regs)
+   return;
+
+   h->ae_algo->ops->get_regs(h, >version, data);
+}
+
 static const struct ethtool_ops hns3vf_ethtool_ops = {
.get_drvinfo = hns3_get_drvinfo,
.get_ringparam = hns3_get_ringparam,
@@ -1103,6 +1124,8 @@ static const struct ethtool_ops hns3_ethtool_ops = {
.set_channels = hns3_set_channels,
.get_coalesce = hns3_get_coalesce,
.set_coalesce = hns3_set_coalesce,
+   .get_regs_len = hns3_get_regs_len,
+   .get_regs = hns3_get_regs,
 };
 
 void hns3_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 3c3159b..2561e7a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -102,6 +102,10 @@ enum hclge_opcode_type {
HCLGE_OPC_STATS_64_BIT  = 0x0030,
HCLGE_OPC_STATS_32_BIT  = 0x0031,
HCLGE_OPC_STATS_MAC = 0x0032,
+
+   HCLGE_OPC_QUERY_REG_NUM = 0x0040,
+   HCLGE_OPC_QUERY_32_BIT_REG  = 0x0041,
+   HCLGE_OPC_QUERY_64_BIT_REG  = 0x0042,
/* Device management command */
 
/* MAC commond */
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 27f0ab6..c3d2cca 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5544,6 +5544,180 @@ static int hclge_set_channels(struct hnae3_handle 
*handle, u32 new_tqps_num)
return ret;
 }
 
+static int hclge_get_regs_num(struct hclge_dev *hdev, u32 *regs_num_32_bit,
+ u32 *regs_num_64_bit)
+{
+   struct hclge_desc desc;
+   u32 total_num;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_QUERY_REG_NUM, true);
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "Query register number cmd failed, ret = %d.\n", ret);
+   return ret;
+   }
+
+   *regs_num_32_bit = le32_to_cpu(desc.data[0]);
+   *regs_num_64_bit = le32_to_cpu(desc.data[1]);
+
+   total_num = *regs_num_32_bit + *regs_num_64_bit;
+   if (!total_num)
+   return -EINVAL;
+
+   return 0;
+}
+
+static int hclge_get_32_bit_regs(struct hclge_dev *hdev, u32 regs_num,
+void *data)
+{
+#define HCLGE_32_BIT_REG_RTN_DATANUM 8
+
+   struct hclge_desc *desc;
+   u32 *reg_val = data;
+   __le32 *desc_data;
+   int cmd_num;
+   int i, k, n;
+   int ret;
+
+   if (regs_num == 0)
+   return 0;
+
+   cmd_num = DIV_ROUND_UP(regs_num + 2, HCLGE_32_BIT_REG_RTN_DATANUM);
+   desc = kcalloc(cmd_num, sizeof(struct hclge_desc), GFP_KERNEL);
+   if (!desc)

[PATCH net-next 1/5] net: hns3: add support for get_regs

[Peng Li]

From: Fuyun Liang 

This patch adds get_regs support for ethtool cmd.

Signed-off-by: Fuyun Liang 
Signed-off-by: Peng Li 
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h|   3 +-
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c |  23 +++
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |   4 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 176 +
 4 files changed, 205 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h 
b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index 634e932..d104ce5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -356,7 +356,8 @@ struct hnae3_ae_ops {
u32 stringset, u8 *data);
int (*get_sset_count)(struct hnae3_handle *handle, int stringset);
 
-   void (*get_regs)(struct hnae3_handle *handle, void *data);
+   void (*get_regs)(struct hnae3_handle *handle, u32 *version,
+void *data);
int (*get_regs_len)(struct hnae3_handle *handle);
 
u32 (*get_rss_key_size)(struct hnae3_handle *handle);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 358f780..1c8b293 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -1063,6 +1063,27 @@ static int hns3_set_coalesce(struct net_device *netdev,
return 0;
 }
 
+static int hns3_get_regs_len(struct net_device *netdev)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo->ops->get_regs_len)
+   return -EOPNOTSUPP;
+
+   return h->ae_algo->ops->get_regs_len(h);
+}
+
+static void hns3_get_regs(struct net_device *netdev,
+ struct ethtool_regs *cmd, void *data)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo->ops->get_regs)
+   return;
+
+   h->ae_algo->ops->get_regs(h, >version, data);
+}
+
 static const struct ethtool_ops hns3vf_ethtool_ops = {
.get_drvinfo = hns3_get_drvinfo,
.get_ringparam = hns3_get_ringparam,
@@ -1103,6 +1124,8 @@ static const struct ethtool_ops hns3_ethtool_ops = {
.set_channels = hns3_set_channels,
.get_coalesce = hns3_get_coalesce,
.set_coalesce = hns3_set_coalesce,
+   .get_regs_len = hns3_get_regs_len,
+   .get_regs = hns3_get_regs,
 };
 
 void hns3_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 3c3159b..2561e7a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -102,6 +102,10 @@ enum hclge_opcode_type {
HCLGE_OPC_STATS_64_BIT  = 0x0030,
HCLGE_OPC_STATS_32_BIT  = 0x0031,
HCLGE_OPC_STATS_MAC = 0x0032,
+
+   HCLGE_OPC_QUERY_REG_NUM = 0x0040,
+   HCLGE_OPC_QUERY_32_BIT_REG  = 0x0041,
+   HCLGE_OPC_QUERY_64_BIT_REG  = 0x0042,
/* Device management command */
 
/* MAC commond */
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 27f0ab6..c3d2cca 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5544,6 +5544,180 @@ static int hclge_set_channels(struct hnae3_handle 
*handle, u32 new_tqps_num)
return ret;
 }
 
+static int hclge_get_regs_num(struct hclge_dev *hdev, u32 *regs_num_32_bit,
+ u32 *regs_num_64_bit)
+{
+   struct hclge_desc desc;
+   u32 total_num;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_QUERY_REG_NUM, true);
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "Query register number cmd failed, ret = %d.\n", ret);
+   return ret;
+   }
+
+   *regs_num_32_bit = le32_to_cpu(desc.data[0]);
+   *regs_num_64_bit = le32_to_cpu(desc.data[1]);
+
+   total_num = *regs_num_32_bit + *regs_num_64_bit;
+   if (!total_num)
+   return -EINVAL;
+
+   return 0;
+}
+
+static int hclge_get_32_bit_regs(struct hclge_dev *hdev, u32 regs_num,
+void *data)
+{
+#define HCLGE_32_BIT_REG_RTN_DATANUM 8
+
+   struct hclge_desc *desc;
+   u32 *reg_val = data;
+   __le32 *desc_data;
+   int cmd_num;
+   int i, k, n;
+   int ret;
+
+   if (regs_num == 0)
+   return 0;
+
+   cmd_num = DIV_ROUND_UP(regs_num + 2, HCLGE_32_BIT_REG_RTN_DATANUM);
+   desc = kcalloc(cmd_num, sizeof(struct hclge_desc), GFP_KERNEL);
+   if (!desc)
+   return -ENOMEM;
+
+   

[PATCH net-next 2/5] net: hns3: add manager table initialization for hardware

[Peng Li]

From: Fuyun Liang 

The manager table is empty by default. If it is not initialized, the
management pkgs like LLDP will be dropped by hardware. Default entries
need to be added to manager table.

Signed-off-by: Fuyun Liang 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  22 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 101 +
 2 files changed, 123 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 2561e7a..1cd28e0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -605,6 +605,28 @@ struct hclge_mac_vlan_mask_entry_cmd {
u8 rsv2[14];
 };
 
+#define HCLGE_MAC_MGR_MASK_VLAN_B  BIT(0)
+#define HCLGE_MAC_MGR_MASK_MAC_B   BIT(1)
+#define HCLGE_MAC_MGR_MASK_ETHERTYPE_B BIT(2)
+#define HCLGE_MAC_ETHERTYPE_LLDP   0x88cc
+
+struct hclge_mac_mgr_tbl_entry_cmd {
+   u8  flags;
+   u8  resp_code;
+   __le16  vlan_tag;
+   __le32  mac_addr_hi32;
+   __le16  mac_addr_lo16;
+   __le16  rsv1;
+   __le16  ethter_type;
+   __le16  egress_port;
+   __le16  egress_queue;
+   u8  sw_port_id_aware;
+   u8  rsv2;
+   u8  i_port_bitmap;
+   u8  i_port_direction;
+   u8  rsv3[2];
+};
+
 #define HCLGE_CFG_MTA_MAC_SEL_S0x0
 #define HCLGE_CFG_MTA_MAC_SEL_MGENMASK(1, 0)
 #define HCLGE_CFG_MTA_MAC_EN_B 0x7
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index c3d2cca..6e64bed 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -392,6 +392,16 @@ static const struct hclge_comm_stats_str 
g_mac_stats_string[] = {
HCLGE_MAC_STATS_FIELD_OFF(mac_rx_send_app_bad_pkt_num)}
 };
 
+static const struct hclge_mac_mgr_tbl_entry_cmd hclge_mgr_table[] = {
+   {
+   .flags = HCLGE_MAC_MGR_MASK_VLAN_B,
+   .ethter_type = cpu_to_le16(HCLGE_MAC_ETHERTYPE_LLDP),
+   .mac_addr_hi32 = cpu_to_le32(htonl(0x0180C200)),
+   .mac_addr_lo16 = cpu_to_le16(htons(0x000E)),
+   .i_port_bitmap = 0x1,
+   },
+};
+
 static int hclge_64_bit_update_stats(struct hclge_dev *hdev)
 {
 #define HCLGE_64_BIT_CMD_NUM 5
@@ -4249,6 +4259,91 @@ int hclge_rm_mc_addr_common(struct hclge_vport *vport,
return status;
 }
 
+static int hclge_get_mac_ethertype_cmd_status(struct hclge_dev *hdev,
+ u16 cmdq_resp, u8 resp_code)
+{
+#define HCLGE_ETHERTYPE_SUCCESS_ADD0
+#define HCLGE_ETHERTYPE_ALREADY_ADD1
+#define HCLGE_ETHERTYPE_MGR_TBL_OVERFLOW   2
+#define HCLGE_ETHERTYPE_KEY_CONFLICT   3
+
+   int return_status;
+
+   if (cmdq_resp) {
+   dev_err(>pdev->dev,
+   "cmdq execute failed for get_mac_ethertype_cmd_status, 
status=%d.\n",
+   cmdq_resp);
+   return -EIO;
+   }
+
+   switch (resp_code) {
+   case HCLGE_ETHERTYPE_SUCCESS_ADD:
+   case HCLGE_ETHERTYPE_ALREADY_ADD:
+   return_status = 0;
+   break;
+   case HCLGE_ETHERTYPE_MGR_TBL_OVERFLOW:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for manager table 
overflow.\n");
+   return_status = -EIO;
+   break;
+   case HCLGE_ETHERTYPE_KEY_CONFLICT:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for key conflict.\n");
+   return_status = -EIO;
+   break;
+   default:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for undefined, code=%d.\n",
+   resp_code);
+   return_status = -EIO;
+   }
+
+   return return_status;
+}
+
+static int hclge_add_mgr_tbl(struct hclge_dev *hdev,
+const struct hclge_mac_mgr_tbl_entry_cmd *req)
+{
+   struct hclge_desc desc;
+   u8 resp_code;
+   u16 retval;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MAC_ETHTYPE_ADD, false);
+   memcpy(desc.data, req, sizeof(struct hclge_mac_mgr_tbl_entry_cmd));
+
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for cmd_send, ret =%d.\n",
+   ret);
+   return ret;
+   }
+
+   resp_code = (le32_to_cpu(desc.data[0]) >> 8) & 0xff;
+   retval = le16_to_cpu(desc.retval);
+
+   return hclge_get_mac_ethertype_cmd_status(hdev, retval, resp_code);

[PATCH net-next 2/5] net: hns3: add manager table initialization for hardware

[Peng Li]

From: Fuyun Liang 

The manager table is empty by default. If it is not initialized, the
management pkgs like LLDP will be dropped by hardware. Default entries
need to be added to manager table.

Signed-off-by: Fuyun Liang 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  22 +
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 101 +
 2 files changed, 123 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 2561e7a..1cd28e0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -605,6 +605,28 @@ struct hclge_mac_vlan_mask_entry_cmd {
u8 rsv2[14];
 };
 
+#define HCLGE_MAC_MGR_MASK_VLAN_B  BIT(0)
+#define HCLGE_MAC_MGR_MASK_MAC_B   BIT(1)
+#define HCLGE_MAC_MGR_MASK_ETHERTYPE_B BIT(2)
+#define HCLGE_MAC_ETHERTYPE_LLDP   0x88cc
+
+struct hclge_mac_mgr_tbl_entry_cmd {
+   u8  flags;
+   u8  resp_code;
+   __le16  vlan_tag;
+   __le32  mac_addr_hi32;
+   __le16  mac_addr_lo16;
+   __le16  rsv1;
+   __le16  ethter_type;
+   __le16  egress_port;
+   __le16  egress_queue;
+   u8  sw_port_id_aware;
+   u8  rsv2;
+   u8  i_port_bitmap;
+   u8  i_port_direction;
+   u8  rsv3[2];
+};
+
 #define HCLGE_CFG_MTA_MAC_SEL_S0x0
 #define HCLGE_CFG_MTA_MAC_SEL_MGENMASK(1, 0)
 #define HCLGE_CFG_MTA_MAC_EN_B 0x7
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index c3d2cca..6e64bed 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -392,6 +392,16 @@ static const struct hclge_comm_stats_str 
g_mac_stats_string[] = {
HCLGE_MAC_STATS_FIELD_OFF(mac_rx_send_app_bad_pkt_num)}
 };
 
+static const struct hclge_mac_mgr_tbl_entry_cmd hclge_mgr_table[] = {
+   {
+   .flags = HCLGE_MAC_MGR_MASK_VLAN_B,
+   .ethter_type = cpu_to_le16(HCLGE_MAC_ETHERTYPE_LLDP),
+   .mac_addr_hi32 = cpu_to_le32(htonl(0x0180C200)),
+   .mac_addr_lo16 = cpu_to_le16(htons(0x000E)),
+   .i_port_bitmap = 0x1,
+   },
+};
+
 static int hclge_64_bit_update_stats(struct hclge_dev *hdev)
 {
 #define HCLGE_64_BIT_CMD_NUM 5
@@ -4249,6 +4259,91 @@ int hclge_rm_mc_addr_common(struct hclge_vport *vport,
return status;
 }
 
+static int hclge_get_mac_ethertype_cmd_status(struct hclge_dev *hdev,
+ u16 cmdq_resp, u8 resp_code)
+{
+#define HCLGE_ETHERTYPE_SUCCESS_ADD0
+#define HCLGE_ETHERTYPE_ALREADY_ADD1
+#define HCLGE_ETHERTYPE_MGR_TBL_OVERFLOW   2
+#define HCLGE_ETHERTYPE_KEY_CONFLICT   3
+
+   int return_status;
+
+   if (cmdq_resp) {
+   dev_err(>pdev->dev,
+   "cmdq execute failed for get_mac_ethertype_cmd_status, 
status=%d.\n",
+   cmdq_resp);
+   return -EIO;
+   }
+
+   switch (resp_code) {
+   case HCLGE_ETHERTYPE_SUCCESS_ADD:
+   case HCLGE_ETHERTYPE_ALREADY_ADD:
+   return_status = 0;
+   break;
+   case HCLGE_ETHERTYPE_MGR_TBL_OVERFLOW:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for manager table 
overflow.\n");
+   return_status = -EIO;
+   break;
+   case HCLGE_ETHERTYPE_KEY_CONFLICT:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for key conflict.\n");
+   return_status = -EIO;
+   break;
+   default:
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for undefined, code=%d.\n",
+   resp_code);
+   return_status = -EIO;
+   }
+
+   return return_status;
+}
+
+static int hclge_add_mgr_tbl(struct hclge_dev *hdev,
+const struct hclge_mac_mgr_tbl_entry_cmd *req)
+{
+   struct hclge_desc desc;
+   u8 resp_code;
+   u16 retval;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_MAC_ETHTYPE_ADD, false);
+   memcpy(desc.data, req, sizeof(struct hclge_mac_mgr_tbl_entry_cmd));
+
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret) {
+   dev_err(>pdev->dev,
+   "add mac ethertype failed for cmd_send, ret =%d.\n",
+   ret);
+   return ret;
+   }
+
+   resp_code = (le32_to_cpu(desc.data[0]) >> 8) & 0xff;
+   retval = le16_to_cpu(desc.retval);
+
+   return hclge_get_mac_ethertype_cmd_status(hdev, retval, resp_code);
+}
+
+static int init_mgr_tbl(struct hclge_dev *hdev)
+{
+   int 

[PATCH net-next 3/5] net: hns3: add ethtool -p support for phy device

[Peng Li]

From: Jian Shen 

Add led location support for phy device. The led will keep blinking
with frequency 2HZ when locating.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h|  2 +
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 12 +++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 88 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h| 12 +++
 4 files changed, 114 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h 
b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index d104ce5..fd06bc7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -405,6 +405,8 @@ struct hnae3_ae_ops {
int (*set_channels)(struct hnae3_handle *handle, u32 new_tqps_num);
void (*get_flowctrl_adv)(struct hnae3_handle *handle,
 u32 *flowctrl_adv);
+   int (*set_led_id)(struct hnae3_handle *handle,
+ enum ethtool_phys_id_state status);
 };
 
 struct hnae3_dcb_ops {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 1c8b293..7410205 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -1084,6 +1084,17 @@ static void hns3_get_regs(struct net_device *netdev,
h->ae_algo->ops->get_regs(h, >version, data);
 }
 
+static int hns3_set_phys_id(struct net_device *netdev,
+   enum ethtool_phys_id_state state)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo || !h->ae_algo->ops || !h->ae_algo->ops->set_led_id)
+   return -EOPNOTSUPP;
+
+   return h->ae_algo->ops->set_led_id(h, state);
+}
+
 static const struct ethtool_ops hns3vf_ethtool_ops = {
.get_drvinfo = hns3_get_drvinfo,
.get_ringparam = hns3_get_ringparam,
@@ -1126,6 +1137,7 @@ static const struct ethtool_ops hns3_ethtool_ops = {
.set_coalesce = hns3_set_coalesce,
.get_regs_len = hns3_get_regs_len,
.get_regs = hns3_get_regs,
+   .set_phys_id = hns3_set_phys_id,
 };
 
 void hns3_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 6e64bed..73caf06 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5819,6 +5819,93 @@ static void hclge_get_regs(struct hnae3_handle *handle, 
u32 *version,
"Get 64 bit register failed, ret = %d.\n", ret);
 }
 
+static int hclge_set_led_status_phy(struct phy_device *phydev, int value)
+{
+   int ret, cur_page;
+
+   mutex_lock(>lock);
+
+   ret = phy_read(phydev, HCLGE_PHY_PAGE_REG);
+   if (ret < 0)
+   goto out;
+   else
+   cur_page = ret;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, HCLGE_PHY_PAGE_LED);
+   if (ret)
+   goto out;
+
+   ret = phy_write(phydev, HCLGE_LED_FC_REG, value);
+   if (ret)
+   goto out;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, cur_page);
+
+out:
+   mutex_unlock(>lock);
+   return ret;
+}
+
+static int hclge_get_led_status_phy(struct phy_device *phydev, int *value)
+{
+   int ret, cur_page;
+
+   mutex_lock(>lock);
+
+   ret = phy_read(phydev, HCLGE_PHY_PAGE_REG);
+   if (ret < 0)
+   goto out;
+   else
+   cur_page = ret;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, HCLGE_PHY_PAGE_LED);
+   if (ret)
+   goto out;
+
+   *value = phy_read(phydev, HCLGE_LED_FC_REG);
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, cur_page);
+
+out:
+   mutex_unlock(>lock);
+   return ret;
+}
+
+static int hclge_set_led_id(struct hnae3_handle *handle,
+   enum ethtool_phys_id_state status)
+{
+#define BLINK_FREQUENCY2
+   struct hclge_vport *vport = hclge_get_vport(handle);
+   struct hclge_dev *hdev = vport->back;
+   struct phy_device *phydev = hdev->hw.mac.phydev;
+   int ret = 0;
+
+   if (!phydev)
+   return -EOPNOTSUPP;
+
+   switch (status) {
+   case ETHTOOL_ID_ACTIVE:
+   ret = hclge_get_led_status_phy(phydev, >phy_led_val);
+   if (ret)
+   return ret;
+   return BLINK_FREQUENCY;
+   case ETHTOOL_ID_ON:
+   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_ON);
+   break;
+   case ETHTOOL_ID_OFF:
+   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_OFF);
+   break;
+   case ETHTOOL_ID_INACTIVE:
+   ret = hclge_set_led_status_phy(phydev, 

[PATCH net-next 3/5] net: hns3: add ethtool -p support for phy device

[Peng Li]

From: Jian Shen 

Add led location support for phy device. The led will keep blinking
with frequency 2HZ when locating.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.h|  2 +
 drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 12 +++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 88 ++
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h| 12 +++
 4 files changed, 114 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h 
b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index d104ce5..fd06bc7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -405,6 +405,8 @@ struct hnae3_ae_ops {
int (*set_channels)(struct hnae3_handle *handle, u32 new_tqps_num);
void (*get_flowctrl_adv)(struct hnae3_handle *handle,
 u32 *flowctrl_adv);
+   int (*set_led_id)(struct hnae3_handle *handle,
+ enum ethtool_phys_id_state status);
 };
 
 struct hnae3_dcb_ops {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 1c8b293..7410205 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -1084,6 +1084,17 @@ static void hns3_get_regs(struct net_device *netdev,
h->ae_algo->ops->get_regs(h, >version, data);
 }
 
+static int hns3_set_phys_id(struct net_device *netdev,
+   enum ethtool_phys_id_state state)
+{
+   struct hnae3_handle *h = hns3_get_handle(netdev);
+
+   if (!h->ae_algo || !h->ae_algo->ops || !h->ae_algo->ops->set_led_id)
+   return -EOPNOTSUPP;
+
+   return h->ae_algo->ops->set_led_id(h, state);
+}
+
 static const struct ethtool_ops hns3vf_ethtool_ops = {
.get_drvinfo = hns3_get_drvinfo,
.get_ringparam = hns3_get_ringparam,
@@ -1126,6 +1137,7 @@ static const struct ethtool_ops hns3_ethtool_ops = {
.set_coalesce = hns3_set_coalesce,
.get_regs_len = hns3_get_regs_len,
.get_regs = hns3_get_regs,
+   .set_phys_id = hns3_set_phys_id,
 };
 
 void hns3_ethtool_set_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 6e64bed..73caf06 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5819,6 +5819,93 @@ static void hclge_get_regs(struct hnae3_handle *handle, 
u32 *version,
"Get 64 bit register failed, ret = %d.\n", ret);
 }
 
+static int hclge_set_led_status_phy(struct phy_device *phydev, int value)
+{
+   int ret, cur_page;
+
+   mutex_lock(>lock);
+
+   ret = phy_read(phydev, HCLGE_PHY_PAGE_REG);
+   if (ret < 0)
+   goto out;
+   else
+   cur_page = ret;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, HCLGE_PHY_PAGE_LED);
+   if (ret)
+   goto out;
+
+   ret = phy_write(phydev, HCLGE_LED_FC_REG, value);
+   if (ret)
+   goto out;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, cur_page);
+
+out:
+   mutex_unlock(>lock);
+   return ret;
+}
+
+static int hclge_get_led_status_phy(struct phy_device *phydev, int *value)
+{
+   int ret, cur_page;
+
+   mutex_lock(>lock);
+
+   ret = phy_read(phydev, HCLGE_PHY_PAGE_REG);
+   if (ret < 0)
+   goto out;
+   else
+   cur_page = ret;
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, HCLGE_PHY_PAGE_LED);
+   if (ret)
+   goto out;
+
+   *value = phy_read(phydev, HCLGE_LED_FC_REG);
+
+   ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, cur_page);
+
+out:
+   mutex_unlock(>lock);
+   return ret;
+}
+
+static int hclge_set_led_id(struct hnae3_handle *handle,
+   enum ethtool_phys_id_state status)
+{
+#define BLINK_FREQUENCY2
+   struct hclge_vport *vport = hclge_get_vport(handle);
+   struct hclge_dev *hdev = vport->back;
+   struct phy_device *phydev = hdev->hw.mac.phydev;
+   int ret = 0;
+
+   if (!phydev)
+   return -EOPNOTSUPP;
+
+   switch (status) {
+   case ETHTOOL_ID_ACTIVE:
+   ret = hclge_get_led_status_phy(phydev, >phy_led_val);
+   if (ret)
+   return ret;
+   return BLINK_FREQUENCY;
+   case ETHTOOL_ID_ON:
+   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_ON);
+   break;
+   case ETHTOOL_ID_OFF:
+   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_OFF);
+   break;
+   case ETHTOOL_ID_INACTIVE:
+   ret = hclge_set_led_status_phy(phydev, hdev->phy_led_val);
+   break;
+   default:
+   

[PATCH net-next 4/5] net: hns3: add ethtool -p support for fiber port

[Peng Li]

From: Jian Shen 

Add led location support for fiber port. The led will keep blinking
when locating.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  20 
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 101 -
 2 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 1cd28e0..122f862 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -227,6 +227,9 @@ enum hclge_opcode_type {
 
/* Mailbox cmd */
HCLGEVF_OPC_MBX_PF_TO_VF= 0x2000,
+
+   /* Led command */
+   HCLGE_OPC_LED_STATUS_CFG= 0xB000,
 };
 
 #define HCLGE_TQP_REG_OFFSET   0x8
@@ -807,6 +810,23 @@ struct hclge_reset_cmd {
 #define HCLGE_NIC_CMQ_DESC_NUM 1024
 #define HCLGE_NIC_CMQ_DESC_NUM_S   3
 
+#define HCLGE_LED_PORT_SPEED_STATE_S   0
+#define HCLGE_LED_PORT_SPEED_STATE_M   GENMASK(5, 0)
+#define HCLGE_LED_ACTIVITY_STATE_S 0
+#define HCLGE_LED_ACTIVITY_STATE_M GENMASK(1, 0)
+#define HCLGE_LED_LINK_STATE_S 0
+#define HCLGE_LED_LINK_STATE_M GENMASK(1, 0)
+#define HCLGE_LED_LOCATE_STATE_S   0
+#define HCLGE_LED_LOCATE_STATE_M   GENMASK(1, 0)
+
+struct hclge_set_led_state_cmd {
+   u8 port_speed_led_config;
+   u8 link_led_config;
+   u8 activity_led_config;
+   u8 locate_led_config;
+   u8 rsv[20];
+};
+
 int hclge_cmd_init(struct hclge_dev *hdev);
 static inline void hclge_write_reg(void __iomem *base, u32 reg, u32 value)
 {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 73caf06..039c90b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5819,6 +5819,34 @@ static void hclge_get_regs(struct hnae3_handle *handle, 
u32 *version,
"Get 64 bit register failed, ret = %d.\n", ret);
 }
 
+static int hclge_set_led_status_sfp(struct hclge_dev *hdev, u8 
speed_led_status,
+   u8 act_led_status, u8 link_led_status,
+   u8 locate_led_status)
+{
+   struct hclge_set_led_state_cmd *req;
+   struct hclge_desc desc;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_LED_STATUS_CFG, false);
+
+   req = (struct hclge_set_led_state_cmd *)desc.data;
+   hnae_set_field(req->port_speed_led_config, HCLGE_LED_PORT_SPEED_STATE_M,
+  HCLGE_LED_PORT_SPEED_STATE_S, speed_led_status);
+   hnae_set_field(req->link_led_config, HCLGE_LED_ACTIVITY_STATE_M,
+  HCLGE_LED_ACTIVITY_STATE_S, act_led_status);
+   hnae_set_field(req->activity_led_config, HCLGE_LED_LINK_STATE_M,
+  HCLGE_LED_LINK_STATE_S, link_led_status);
+   hnae_set_field(req->locate_led_config, HCLGE_LED_LOCATE_STATE_M,
+  HCLGE_LED_LOCATE_STATE_S, locate_led_status);
+
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret)
+   dev_err(>pdev->dev,
+   "Send set led state cmd error, ret =%d\n", ret);
+
+   return ret;
+}
+
 static int hclge_set_led_status_phy(struct phy_device *phydev, int value)
 {
int ret, cur_page;
@@ -5871,6 +5899,12 @@ static int hclge_get_led_status_phy(struct phy_device 
*phydev, int *value)
return ret;
 }
 
+enum hclge_led_status {
+   HCLGE_LED_OFF,
+   HCLGE_LED_ON,
+   HCLGE_LED_NO_CHANGE = 0xFF,
+};
+
 static int hclge_set_led_id(struct hnae3_handle *handle,
enum ethtool_phys_id_state status)
 {
@@ -5880,27 +5914,52 @@ static int hclge_set_led_id(struct hnae3_handle *handle,
struct phy_device *phydev = hdev->hw.mac.phydev;
int ret = 0;
 
-   if (!phydev)
-   return -EOPNOTSUPP;
-
-   switch (status) {
-   case ETHTOOL_ID_ACTIVE:
-   ret = hclge_get_led_status_phy(phydev, >phy_led_val);
-   if (ret)
-   return ret;
-   return BLINK_FREQUENCY;
-   case ETHTOOL_ID_ON:
-   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_ON);
-   break;
-   case ETHTOOL_ID_OFF:
-   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_OFF);
-   break;
-   case ETHTOOL_ID_INACTIVE:
-   ret = hclge_set_led_status_phy(phydev, hdev->phy_led_val);
-   break;
-   default:
-   ret = -EINVAL;
-   break;
+   if (phydev) {
+   switch (status) {
+   case ETHTOOL_ID_ACTIVE:
+   ret = hclge_get_led_status_phy(phydev,
+ 

[PATCH net-next 4/5] net: hns3: add ethtool -p support for fiber port

[Peng Li]

From: Jian Shen 

Add led location support for fiber port. The led will keep blinking
when locating.

Signed-off-by: Jian Shen 
Signed-off-by: Peng Li 
---
 .../net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h |  20 
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c| 101 -
 2 files changed, 100 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 1cd28e0..122f862 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -227,6 +227,9 @@ enum hclge_opcode_type {
 
/* Mailbox cmd */
HCLGEVF_OPC_MBX_PF_TO_VF= 0x2000,
+
+   /* Led command */
+   HCLGE_OPC_LED_STATUS_CFG= 0xB000,
 };
 
 #define HCLGE_TQP_REG_OFFSET   0x8
@@ -807,6 +810,23 @@ struct hclge_reset_cmd {
 #define HCLGE_NIC_CMQ_DESC_NUM 1024
 #define HCLGE_NIC_CMQ_DESC_NUM_S   3
 
+#define HCLGE_LED_PORT_SPEED_STATE_S   0
+#define HCLGE_LED_PORT_SPEED_STATE_M   GENMASK(5, 0)
+#define HCLGE_LED_ACTIVITY_STATE_S 0
+#define HCLGE_LED_ACTIVITY_STATE_M GENMASK(1, 0)
+#define HCLGE_LED_LINK_STATE_S 0
+#define HCLGE_LED_LINK_STATE_M GENMASK(1, 0)
+#define HCLGE_LED_LOCATE_STATE_S   0
+#define HCLGE_LED_LOCATE_STATE_M   GENMASK(1, 0)
+
+struct hclge_set_led_state_cmd {
+   u8 port_speed_led_config;
+   u8 link_led_config;
+   u8 activity_led_config;
+   u8 locate_led_config;
+   u8 rsv[20];
+};
+
 int hclge_cmd_init(struct hclge_dev *hdev);
 static inline void hclge_write_reg(void __iomem *base, u32 reg, u32 value)
 {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c 
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 73caf06..039c90b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5819,6 +5819,34 @@ static void hclge_get_regs(struct hnae3_handle *handle, 
u32 *version,
"Get 64 bit register failed, ret = %d.\n", ret);
 }
 
+static int hclge_set_led_status_sfp(struct hclge_dev *hdev, u8 
speed_led_status,
+   u8 act_led_status, u8 link_led_status,
+   u8 locate_led_status)
+{
+   struct hclge_set_led_state_cmd *req;
+   struct hclge_desc desc;
+   int ret;
+
+   hclge_cmd_setup_basic_desc(, HCLGE_OPC_LED_STATUS_CFG, false);
+
+   req = (struct hclge_set_led_state_cmd *)desc.data;
+   hnae_set_field(req->port_speed_led_config, HCLGE_LED_PORT_SPEED_STATE_M,
+  HCLGE_LED_PORT_SPEED_STATE_S, speed_led_status);
+   hnae_set_field(req->link_led_config, HCLGE_LED_ACTIVITY_STATE_M,
+  HCLGE_LED_ACTIVITY_STATE_S, act_led_status);
+   hnae_set_field(req->activity_led_config, HCLGE_LED_LINK_STATE_M,
+  HCLGE_LED_LINK_STATE_S, link_led_status);
+   hnae_set_field(req->locate_led_config, HCLGE_LED_LOCATE_STATE_M,
+  HCLGE_LED_LOCATE_STATE_S, locate_led_status);
+
+   ret = hclge_cmd_send(>hw, , 1);
+   if (ret)
+   dev_err(>pdev->dev,
+   "Send set led state cmd error, ret =%d\n", ret);
+
+   return ret;
+}
+
 static int hclge_set_led_status_phy(struct phy_device *phydev, int value)
 {
int ret, cur_page;
@@ -5871,6 +5899,12 @@ static int hclge_get_led_status_phy(struct phy_device 
*phydev, int *value)
return ret;
 }
 
+enum hclge_led_status {
+   HCLGE_LED_OFF,
+   HCLGE_LED_ON,
+   HCLGE_LED_NO_CHANGE = 0xFF,
+};
+
 static int hclge_set_led_id(struct hnae3_handle *handle,
enum ethtool_phys_id_state status)
 {
@@ -5880,27 +5914,52 @@ static int hclge_set_led_id(struct hnae3_handle *handle,
struct phy_device *phydev = hdev->hw.mac.phydev;
int ret = 0;
 
-   if (!phydev)
-   return -EOPNOTSUPP;
-
-   switch (status) {
-   case ETHTOOL_ID_ACTIVE:
-   ret = hclge_get_led_status_phy(phydev, >phy_led_val);
-   if (ret)
-   return ret;
-   return BLINK_FREQUENCY;
-   case ETHTOOL_ID_ON:
-   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_ON);
-   break;
-   case ETHTOOL_ID_OFF:
-   ret = hclge_set_led_status_phy(phydev, HCLGE_LED_FORCE_OFF);
-   break;
-   case ETHTOOL_ID_INACTIVE:
-   ret = hclge_set_led_status_phy(phydev, hdev->phy_led_val);
-   break;
-   default:
-   ret = -EINVAL;
-   break;
+   if (phydev) {
+   switch (status) {
+   case ETHTOOL_ID_ACTIVE:
+   ret = hclge_get_led_status_phy(phydev,
+  >phy_led_val);
+   

Re: blk-mq: don't dispatch request in blk_mq_request_direct_issue if queue is busy

[Laurence Oberman]

On Wed, 2018-01-17 at 23:36 -0500, Mike Snitzer wrote:
> On Wed, Jan 17 2018 at 11:06pm -0500,
> Ming Lei  wrote:
> 
> > If we run into blk_mq_request_direct_issue(), when queue is busy,
> > we
> > don't want to dispatch this request into hctx->dispatch_list, and
> > what we need to do is to return the queue busy info to caller, so
> > that caller can deal with it well.
> > 
> > Fixes: 396eaf21ee ("blk-mq: improve DM's blk-mq IO merging via
> > blk_insert_cloned_request feedback")
> > Reported-by: Laurence Oberman 
> > Reviewed-by: Mike Snitzer 
> > Signed-off-by: Ming Lei 
> > ---
> >  block/blk-mq.c | 22 ++
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 4d4af8d712da..1af7fa70993b 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -1856,15 +1856,6 @@ static blk_status_t
> > __blk_mq_issue_directly(struct blk_mq_hw_ctx *hctx,
> >     return ret;
> >  }
> >  
> > -static void __blk_mq_fallback_to_insert(struct request *rq,
> > -   bool run_queue, bool
> > bypass_insert)
> > -{
> > -   if (!bypass_insert)
> > -   blk_mq_sched_insert_request(rq, false, run_queue,
> > false);
> > -   else
> > -   blk_mq_request_bypass_insert(rq, run_queue);
> > -}
> > -
> >  static blk_status_t __blk_mq_try_issue_directly(struct
> > blk_mq_hw_ctx *hctx,
> >     struct request
> > *rq,
> >     blk_qc_t *cookie,
> > @@ -1873,9 +1864,16 @@ static blk_status_t
> > __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> >     struct request_queue *q = rq->q;
> >     bool run_queue = true;
> >  
> > -   /* RCU or SRCU read lock is needed before checking
> > quiesced flag */
> > +   /*
> > +    * RCU or SRCU read lock is needed before checking
> > quiesced flag.
> > +    *
> > +    * When queue is stopped or quiesced, ignore
> > 'bypass_insert' from
> > +    * blk_mq_request_direct_issue(), and return BLK_STS_OK to
> > caller,
> > +    * and avoid driver to try to dispatch again.
> > +    */
> >     if (blk_mq_hctx_stopped(hctx) || blk_queue_quiesced(q)) {
> >     run_queue = false;
> > +   bypass_insert = false;
> >     goto insert;
> >     }
> >  
> > @@ -1892,10 +1890,10 @@ static blk_status_t
> > __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> >  
> >     return __blk_mq_issue_directly(hctx, rq, cookie);
> >  insert:
> > -   __blk_mq_fallback_to_insert(rq, run_queue, bypass_insert);
> >     if (bypass_insert)
> >     return BLK_STS_RESOURCE;
> >  
> > +   blk_mq_sched_insert_request(rq, false, run_queue, false);
> >     return BLK_STS_OK;
> >  }
> 
> OK so you're just leveraging blk_mq_sched_insert_request()'s
> ability to resort to__blk_mq_insert_request() if !q->elevator.

I tested this against Mike's latest combined tree and its stable.
This fixes the list corruption issue.
Many Thanks Ming and Mike.

I will apply it to Bart's latest SRP/SRPT tree tomorrow as its very
late here but it will clearly fix the issue in Bart's tree too.

Tested-by: Laurence Oberman 

Re: blk-mq: don't dispatch request in blk_mq_request_direct_issue if queue is busy

[Laurence Oberman]

On Wed, 2018-01-17 at 23:36 -0500, Mike Snitzer wrote:
> On Wed, Jan 17 2018 at 11:06pm -0500,
> Ming Lei  wrote:
> 
> > If we run into blk_mq_request_direct_issue(), when queue is busy,
> > we
> > don't want to dispatch this request into hctx->dispatch_list, and
> > what we need to do is to return the queue busy info to caller, so
> > that caller can deal with it well.
> > 
> > Fixes: 396eaf21ee ("blk-mq: improve DM's blk-mq IO merging via
> > blk_insert_cloned_request feedback")
> > Reported-by: Laurence Oberman 
> > Reviewed-by: Mike Snitzer 
> > Signed-off-by: Ming Lei 
> > ---
> >  block/blk-mq.c | 22 ++
> >  1 file changed, 10 insertions(+), 12 deletions(-)
> > 
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 4d4af8d712da..1af7fa70993b 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -1856,15 +1856,6 @@ static blk_status_t
> > __blk_mq_issue_directly(struct blk_mq_hw_ctx *hctx,
> >     return ret;
> >  }
> >  
> > -static void __blk_mq_fallback_to_insert(struct request *rq,
> > -   bool run_queue, bool
> > bypass_insert)
> > -{
> > -   if (!bypass_insert)
> > -   blk_mq_sched_insert_request(rq, false, run_queue,
> > false);
> > -   else
> > -   blk_mq_request_bypass_insert(rq, run_queue);
> > -}
> > -
> >  static blk_status_t __blk_mq_try_issue_directly(struct
> > blk_mq_hw_ctx *hctx,
> >     struct request
> > *rq,
> >     blk_qc_t *cookie,
> > @@ -1873,9 +1864,16 @@ static blk_status_t
> > __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> >     struct request_queue *q = rq->q;
> >     bool run_queue = true;
> >  
> > -   /* RCU or SRCU read lock is needed before checking
> > quiesced flag */
> > +   /*
> > +    * RCU or SRCU read lock is needed before checking
> > quiesced flag.
> > +    *
> > +    * When queue is stopped or quiesced, ignore
> > 'bypass_insert' from
> > +    * blk_mq_request_direct_issue(), and return BLK_STS_OK to
> > caller,
> > +    * and avoid driver to try to dispatch again.
> > +    */
> >     if (blk_mq_hctx_stopped(hctx) || blk_queue_quiesced(q)) {
> >     run_queue = false;
> > +   bypass_insert = false;
> >     goto insert;
> >     }
> >  
> > @@ -1892,10 +1890,10 @@ static blk_status_t
> > __blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
> >  
> >     return __blk_mq_issue_directly(hctx, rq, cookie);
> >  insert:
> > -   __blk_mq_fallback_to_insert(rq, run_queue, bypass_insert);
> >     if (bypass_insert)
> >     return BLK_STS_RESOURCE;
> >  
> > +   blk_mq_sched_insert_request(rq, false, run_queue, false);
> >     return BLK_STS_OK;
> >  }
> 
> OK so you're just leveraging blk_mq_sched_insert_request()'s
> ability to resort to__blk_mq_insert_request() if !q->elevator.

I tested this against Mike's latest combined tree and its stable.
This fixes the list corruption issue.
Many Thanks Ming and Mike.

I will apply it to Bart's latest SRP/SRPT tree tomorrow as its very
late here but it will clearly fix the issue in Bart's tree too.

Tested-by: Laurence Oberman 

Re: [PATCH v5 12/44] clk: davinci: Add platform information for TI DA850 PSC

[Sekhar Nori]

On Thursday 18 January 2018 12:38 AM, David Lechner wrote:
> It is looking like this:
> 
> 
> static const struct davinci_psc_clkdev_info emfia_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "ti-aemif"),
> LPSC_CLKDEV("aemif",    "davinci-nand.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info spi0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "spi_davinci.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info mmcsd0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "da830-mmc.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info uart0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "serial8250.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info arm_clkdev[] __initconst = {
> /*
>  * REVISIT: cpufreq-davinci should be modified to use dev_id and drop
>  * use of con_id.
>  */
> LPSC_CLKDEV("arm",    NULL),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info dsp_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "davinci-rproc.0"),
> { }
> };
> 
> static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
> LPSC(0,  0, tpcc0,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(1,  0, tptc0,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(2,  0, tptc1,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(3,  0, emifa,   async1,   emfia_clkdev,  0),
> LPSC(4,  0, spi0,    pll0_sysclk2, spi0_clkdev,   0),
> LPSC(5,  0, mmcsd0,  pll0_sysclk2, mmcsd0_clkdev, 0),
> LPSC(6,  0, aintc,   pll0_sysclk4, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(7,  0, arm_rom, pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(9,  0, uart0,   pll0_sysclk2, uart0_clkdev,  0),
> LPSC(13, 0, pruss,   pll0_sysclk2, NULL,  0),
> LPSC(14, 0, arm, pll0_sysclk6, arm_clkdev,    LPSC_ALWAYS_ENABLED |
>   LPSC_ARM_RATE),
> LPSC(15, 1, dsp, pll0_sysclk1, dsp_clkdev,    LPSC_FORCE |
>   LPSC_LOCAL_RESET),
> { }
> };

This looks good to me!

Regards,
Sekhar

Re: [PATCH v5 12/44] clk: davinci: Add platform information for TI DA850 PSC

[Sekhar Nori]

On Thursday 18 January 2018 12:38 AM, David Lechner wrote:
> It is looking like this:
> 
> 
> static const struct davinci_psc_clkdev_info emfia_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "ti-aemif"),
> LPSC_CLKDEV("aemif",    "davinci-nand.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info spi0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "spi_davinci.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info mmcsd0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "da830-mmc.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info uart0_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "serial8250.0"),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info arm_clkdev[] __initconst = {
> /*
>  * REVISIT: cpufreq-davinci should be modified to use dev_id and drop
>  * use of con_id.
>  */
> LPSC_CLKDEV("arm",    NULL),
> { }
> };
> 
> static const struct davinci_psc_clkdev_info dsp_clkdev[] __initconst = {
> LPSC_CLKDEV(NULL,    "davinci-rproc.0"),
> { }
> };
> 
> static const struct davinci_psc_clk_info da850_psc0_info[] __initconst = {
> LPSC(0,  0, tpcc0,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(1,  0, tptc0,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(2,  0, tptc1,   pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(3,  0, emifa,   async1,   emfia_clkdev,  0),
> LPSC(4,  0, spi0,    pll0_sysclk2, spi0_clkdev,   0),
> LPSC(5,  0, mmcsd0,  pll0_sysclk2, mmcsd0_clkdev, 0),
> LPSC(6,  0, aintc,   pll0_sysclk4, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(7,  0, arm_rom, pll0_sysclk2, NULL,  LPSC_ALWAYS_ENABLED),
> LPSC(9,  0, uart0,   pll0_sysclk2, uart0_clkdev,  0),
> LPSC(13, 0, pruss,   pll0_sysclk2, NULL,  0),
> LPSC(14, 0, arm, pll0_sysclk6, arm_clkdev,    LPSC_ALWAYS_ENABLED |
>   LPSC_ARM_RATE),
> LPSC(15, 1, dsp, pll0_sysclk1, dsp_clkdev,    LPSC_FORCE |
>   LPSC_LOCAL_RESET),
> { }
> };

This looks good to me!

Regards,
Sekhar

Re: [PATCH 0/7] PM /Domain/OPP: Add support to get performance state from DT

[Viresh Kumar]

On 22-12-17, 12:56, Viresh Kumar wrote:
> Hi,
> 
> Now that the DT bindings [1] are already Reviewed/Acked by respective
> maintainers, here is the code to start using them.
> 
> The first two patches provide helpers in the OPP core, [3-5]/7 update
> the PM domain core to start supporting domain OPP tables, etc, 6/7
> updates the OPP core to use the new callback provided by the PM domains
> to get performance state and the last one removes the unused helpers
> now.
> 
> This is tested on Hikey620 and works just fine.

Ping !

-- 
viresh

Re: [PATCH 0/7] PM /Domain/OPP: Add support to get performance state from DT

[Viresh Kumar]

On 22-12-17, 12:56, Viresh Kumar wrote:
> Hi,
> 
> Now that the DT bindings [1] are already Reviewed/Acked by respective
> maintainers, here is the code to start using them.
> 
> The first two patches provide helpers in the OPP core, [3-5]/7 update
> the PM domain core to start supporting domain OPP tables, etc, 6/7
> updates the OPP core to use the new callback provided by the PM domains
> to get performance state and the last one removes the unused helpers
> now.
> 
> This is tested on Hikey620 and works just fine.

Ping !

-- 
viresh

Re: [PATCH v2] KVM: s390: wire up bpb feature

[Martin Schwidefsky]

On Wed, 17 Jan 2018 22:43:24 +0100
Christian Borntraeger  wrote:

> Conny can you review and ack as well?
> 
> Paolo, Radim,
> 
> As the other patches need to sync on the ifetch/nospec/gmb naming I have 
> changed my mind. :-)
> This patch is independent from the other patches (as it just provides the 
> guest facilities not caring
> about what the host does).
> 
> It seems that you do a kvm pull request for 4.15 anyway (for power), so it 
> might make sense to
> apply this patch as well for 4.15. this will make it easier to also upstream 
> the QEMU part in time
> as we need the uabi interfaces.

Indeed, there is no real dependency. I am thinking about doing a split of my 
patches as well,
everything but the gmb/nospec_xxx/ifetch/isync/whatever part. The prctl part 
needs some more
discussion, as will prepare a patch. Or two.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH v2] KVM: s390: wire up bpb feature

[Martin Schwidefsky]

On Wed, 17 Jan 2018 22:43:24 +0100
Christian Borntraeger  wrote:

> Conny can you review and ack as well?
> 
> Paolo, Radim,
> 
> As the other patches need to sync on the ifetch/nospec/gmb naming I have 
> changed my mind. :-)
> This patch is independent from the other patches (as it just provides the 
> guest facilities not caring
> about what the host does).
> 
> It seems that you do a kvm pull request for 4.15 anyway (for power), so it 
> might make sense to
> apply this patch as well for 4.15. this will make it easier to also upstream 
> the QEMU part in time
> as we need the uabi interfaces.

Indeed, there is no real dependency. I am thinking about doing a split of my 
patches as well,
everything but the gmb/nospec_xxx/ifetch/isync/whatever part. The prctl part 
needs some more
discussion, as will prepare a patch. Or two.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH V4 1/2] nvme: add NVME_CTRL_RESET_PREPARE state

[James Smart]

Thanks jianchoa. This helped.

On 1/17/2018 7:13 PM, jianchao.wang wrote:

Actually, this patchset is to fix a issue in nvme_timeout.
Please consider the following scenario.

nvme_reset_ctrl
 -> set state to RESETTING
 -> queue reset_work
 (scheduling)
  nvme_reset_work  -> nvme_dev_disable
 -> quiesce queues
   -> nvme_cancel_request
  on outstanding requests
  _boundary_
   -> nvme initializing (may issue request on adminq)

Before the boundary, not only quiesce the queues, but only cancel all the 
outstanding requests.

A request could expire when the ctrl state is RESETTING.
  - If the timeout occur before the _boundary_, the expired requests are from 
the previous work.
  - Otherwise, the expired requests are from the controller initializing 
procedure, such as sending cq/sq
create commands to adminq to setup io queues.
In current implementation, nvme_timeout cannot identify the _boundary_ so only 
handles second case above.


So what you've described very well is the pci adapter and the fact that 
it doesn't use a RECONNECTING state when it starts to reinit the 
controller like rdma/fc does.  Note: we had left it that way as a 
"grandfathering" as the code already existed and we didn't see an issue 
just leaving the reinit in the resetting handler.






So in the patch, RESETTING in nvme-fc/rdma is changed to RESET_PREPARE. Then we 
get:
nvme-fc/rdma RESET_PREPARE -> RECONNECTING -> LIVE
nvme-pci RESET_PREPARE -> RESETTING-> LIVE


Right - so another way to look at this is you renamed RESETTING to 
RESET_PREPARE and added a new RESETTING state in the nvme-pci when in 
reinits.  Why not simply have the nvme-pci transport transition to 
RECONNECTING like the other transports. No new states, semantics are all 
the same.


Here's what the responsibility of the states are:
RESETTING:
-quiescing the blk-mq queues os errors don't bubble back to callees and 
new io is suspended
-terminating the link-side association: resets the controller via 
register access/SET_PROPERTY, deletes connections/queues, cleans out 
active io and lets them queue for resending, etc.
-end result is nvme block device is present, but idle, and no active 
controller on the link side  (link being a transport specific link such 
as pci, or ethernet/ib for rdma or fc link).


RECONNECTING:
- "establish an association at the transport" on PCI this is immediate - 
you can either talk to the pci function or you can't. With rdma/fc we 
send transport traffic to create an admin q connection.
- if association succeeded: the controller is init'd via register 
accesses/fabric GET/SET_PROPERTIES and admin-queue command, io 
connections/queues created, blk-mq queues unquiesced, and finally 
transition to NVME_CTRL_LIVE.
- if association failed: check controller timeout., if not exceeded, 
schedule timer and retry later.  With pci, this could cover the 
temporary loss of the pci function access (a hot plug event) while 
keeping the nvme blk device live in the system.


It matches what you are describing but using what we already have in 
place - with the only difference being the addition of your "boundary" 
by adding the RECONNECTING state to nvme-pci.






I don't believe RESETTING and RECONNECTING are that similar - unless - you are looking at 
that "reinit" part that we left grandfathered into PCI.

Yes. I have got the point. Thanks for your directive.

Both nvme-pc and nvme-fc/rdma have "shutdown" part to tear down 
queues/connects, quiesce queues, cancel outstanding requests...
We could call this part as "shutdowning" as well as the scheduling gap.
Because the difference of initializing between the nvme-pci and nvme-fc/rdma,  we could 
call "reiniting" for nvme-pci and
call "reconnecting" for nvme-fc/rdma


I don't think we need different states for the two. The actions taken 
are really very similar. How they do the actions varies slightly, but 
not what they are trying to do.   Thus, I'd prefer we keep the existing 
RECONNECTING state and use it in nvme-pci as well. I'm open to renaming 
it if there's something about the name that is not agreeable.



-- james

Re: [PATCH V4 1/2] nvme: add NVME_CTRL_RESET_PREPARE state

[James Smart]

Thanks jianchoa. This helped.

On 1/17/2018 7:13 PM, jianchao.wang wrote:

Actually, this patchset is to fix a issue in nvme_timeout.
Please consider the following scenario.

nvme_reset_ctrl
 -> set state to RESETTING
 -> queue reset_work
 (scheduling)
  nvme_reset_work  -> nvme_dev_disable
 -> quiesce queues
   -> nvme_cancel_request
  on outstanding requests
  _boundary_
   -> nvme initializing (may issue request on adminq)

Before the boundary, not only quiesce the queues, but only cancel all the 
outstanding requests.

A request could expire when the ctrl state is RESETTING.
  - If the timeout occur before the _boundary_, the expired requests are from 
the previous work.
  - Otherwise, the expired requests are from the controller initializing 
procedure, such as sending cq/sq
create commands to adminq to setup io queues.
In current implementation, nvme_timeout cannot identify the _boundary_ so only 
handles second case above.


So what you've described very well is the pci adapter and the fact that 
it doesn't use a RECONNECTING state when it starts to reinit the 
controller like rdma/fc does.  Note: we had left it that way as a 
"grandfathering" as the code already existed and we didn't see an issue 
just leaving the reinit in the resetting handler.






So in the patch, RESETTING in nvme-fc/rdma is changed to RESET_PREPARE. Then we 
get:
nvme-fc/rdma RESET_PREPARE -> RECONNECTING -> LIVE
nvme-pci RESET_PREPARE -> RESETTING-> LIVE


Right - so another way to look at this is you renamed RESETTING to 
RESET_PREPARE and added a new RESETTING state in the nvme-pci when in 
reinits.  Why not simply have the nvme-pci transport transition to 
RECONNECTING like the other transports. No new states, semantics are all 
the same.


Here's what the responsibility of the states are:
RESETTING:
-quiescing the blk-mq queues os errors don't bubble back to callees and 
new io is suspended
-terminating the link-side association: resets the controller via 
register access/SET_PROPERTY, deletes connections/queues, cleans out 
active io and lets them queue for resending, etc.
-end result is nvme block device is present, but idle, and no active 
controller on the link side  (link being a transport specific link such 
as pci, or ethernet/ib for rdma or fc link).


RECONNECTING:
- "establish an association at the transport" on PCI this is immediate - 
you can either talk to the pci function or you can't. With rdma/fc we 
send transport traffic to create an admin q connection.
- if association succeeded: the controller is init'd via register 
accesses/fabric GET/SET_PROPERTIES and admin-queue command, io 
connections/queues created, blk-mq queues unquiesced, and finally 
transition to NVME_CTRL_LIVE.
- if association failed: check controller timeout., if not exceeded, 
schedule timer and retry later.  With pci, this could cover the 
temporary loss of the pci function access (a hot plug event) while 
keeping the nvme blk device live in the system.


It matches what you are describing but using what we already have in 
place - with the only difference being the addition of your "boundary" 
by adding the RECONNECTING state to nvme-pci.






I don't believe RESETTING and RECONNECTING are that similar - unless - you are looking at 
that "reinit" part that we left grandfathered into PCI.

Yes. I have got the point. Thanks for your directive.

Both nvme-pc and nvme-fc/rdma have "shutdown" part to tear down 
queues/connects, quiesce queues, cancel outstanding requests...
We could call this part as "shutdowning" as well as the scheduling gap.
Because the difference of initializing between the nvme-pci and nvme-fc/rdma,  we could 
call "reiniting" for nvme-pci and
call "reconnecting" for nvme-fc/rdma


I don't think we need different states for the two. The actions taken 
are really very similar. How they do the actions varies slightly, but 
not what they are trying to do.   Thus, I'd prefer we keep the existing 
RECONNECTING state and use it in nvme-pci as well. I'm open to renaming 
it if there's something about the name that is not agreeable.



-- james

Re: [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 06:59:59PM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 12.01.2018 um 16:39 schrieb Johan Hovold :
> > 
> >> Let's restart this discussion and focus on the main roadblock (others
> >> are minor details which can be sorted out later).
> >> 
> >> If it feels like a hack, the key issue seems to me to be the choice of
> >> the API to present the GPS data to user space. Right?
> > 
> > Or even more fundamentally, does this belong in the kernel at all?
> 
> Yes, that can be questioned of course. It was questioned and discussed
> several times and I thought the answer was a clear yes. But let's reiterate.
> 
> > Also it seems at least part of your specific problem is that you have
> > failed to wire up the WAKEUP pin of the W2SG0004/84 properly,
> 
> The w2sg0004 has no wakeup pin. At least I can't find one in the data sheet.

I should have said w2sg0084 above, which is the only datasheet I have found.

> The two pins you refer to from the 0084 data sheet are called BootSelect0/1
> in the 0004 and have a different function.
> 
> To be clear, we did not fail to wire it up. We did the design before the
> 0084 was announced and available. We just had to swap in the 0084 into
> existing PCBs during production because the 0004 became EOL. Otherwise
> we would probably still use the 0004 without WAKEUP output.
> 
> To make it worse, we have no documentation for an individual board if
> an 0004 or 0084 chip is installed and there is no means how a software
> can find out which one it is talking to (especially before properly
> powering on). Therefore we can not even provide two different device
> trees or drivers or whatever, unless we ask people to open their device
> and look on the chip. Quite crazy wrt. user-friendlyness of software
> installation in 2018...
> 
> Therefore, a driver must be capable to handle both chips in the same way,
> with minimalistic assumptions, even if the 0084 could provide a direct
> signal to make it easier than using serdev to monitor the data stream.

Fair enough.

> >  which then
> > forces you to look at the data stream to determine the power state of
> > the chip. Judging from a quick look at the GTA04 schematics it seems
> > you've even connected the WAKEUP output to the 1V8_OUT output?!
> 
> No. You failed to see that this is an optional 0R, which is not installed.
> The 0R on pin 7 (BootSelect1) to GND was removed when we did switch from
> 0004 to 0084. Pin 6 (BootSelect0/WAKEUP) was never connected.

Ok.

> > The kernel is probably not the place to be working around issues like
> > that,

> > even if serdev at least allows for such hacks to be fairly
> > isolated in drivers (unlike some of the earlier proposals touching core
> > code).
> 
> Please tell me why there are so many hacks for hardware issues in certain
> drivers. Any why those are good and this one (if it is one at all) is not.

Hacks are never good, but sometimes needed. But we should try to keep
them contained in drivers rather than allow them to spread to core code,
was what I was trying to say above.

> What I can learn from your discussion is that it might be considerable
> to add an optional gpio for the 0084 WAKEUP and add some logic to
> support users who have or will have that pin connected.
> 
> But even then we would need a driver to handle this gpio and issue
> an on/off impulse on the other to switch states. It would be a different
> driver (variant - maybe some CONFIG option or handled by code), but not
> "no driver".

Having a WAKEUP signal would allow for a more straight-forward
implementation, be it in the kernel or in user space.

> >> I see three reasonable options how this presentation can be done:
> >> 
> >> 1. char device
> >> 2. tty device
> >> 3. some new gps interface API (similar to network, bluetooth interfaces)
> >> 4. no driver and use the UART tty directly
> >> 
> >> Pros and cons:
> > 
> >> 4. no driver and use UART directly
> >> + a non-solution seems to be attractive
> >> - must turn on/off chip by gpio hacks from user-space
> > 
> > I'm not sure that would amount to more of hack then doing it in the
> > kernel would.
> 
> It might not be big effort in the user-space code/scripts.
> 
> But much effort to convince all the plethora of user-space client maintainers
> to integrate something. And have them roll out. And have distributions take 
> it.
> And have users upgrade to it. 5 years later...
> 
> Do you think it is easier to convince them than you? They usually assume a
> power management issue should be solved by the kernel driver.
> 
> That is what Andreas did remark as motivation: provide a solution
> for *existing* user spaces.

I understand that this is what you want, but that in itself is not a
sufficient reason to put something in the kernel.

> >> - can not guarantee (!) to power off the chip if the last user-space
> >> process using it is killed (which is essential for power-management of
> >> a handheld, battery operated 

Re: [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver

[Johan Hovold]

On Fri, Jan 12, 2018 at 06:59:59PM +0100, H. Nikolaus Schaller wrote:
> Hi Johan,
> 
> > Am 12.01.2018 um 16:39 schrieb Johan Hovold :
> > 
> >> Let's restart this discussion and focus on the main roadblock (others
> >> are minor details which can be sorted out later).
> >> 
> >> If it feels like a hack, the key issue seems to me to be the choice of
> >> the API to present the GPS data to user space. Right?
> > 
> > Or even more fundamentally, does this belong in the kernel at all?
> 
> Yes, that can be questioned of course. It was questioned and discussed
> several times and I thought the answer was a clear yes. But let's reiterate.
> 
> > Also it seems at least part of your specific problem is that you have
> > failed to wire up the WAKEUP pin of the W2SG0004/84 properly,
> 
> The w2sg0004 has no wakeup pin. At least I can't find one in the data sheet.

I should have said w2sg0084 above, which is the only datasheet I have found.

> The two pins you refer to from the 0084 data sheet are called BootSelect0/1
> in the 0004 and have a different function.
> 
> To be clear, we did not fail to wire it up. We did the design before the
> 0084 was announced and available. We just had to swap in the 0084 into
> existing PCBs during production because the 0004 became EOL. Otherwise
> we would probably still use the 0004 without WAKEUP output.
> 
> To make it worse, we have no documentation for an individual board if
> an 0004 or 0084 chip is installed and there is no means how a software
> can find out which one it is talking to (especially before properly
> powering on). Therefore we can not even provide two different device
> trees or drivers or whatever, unless we ask people to open their device
> and look on the chip. Quite crazy wrt. user-friendlyness of software
> installation in 2018...
> 
> Therefore, a driver must be capable to handle both chips in the same way,
> with minimalistic assumptions, even if the 0084 could provide a direct
> signal to make it easier than using serdev to monitor the data stream.

Fair enough.

> >  which then
> > forces you to look at the data stream to determine the power state of
> > the chip. Judging from a quick look at the GTA04 schematics it seems
> > you've even connected the WAKEUP output to the 1V8_OUT output?!
> 
> No. You failed to see that this is an optional 0R, which is not installed.
> The 0R on pin 7 (BootSelect1) to GND was removed when we did switch from
> 0004 to 0084. Pin 6 (BootSelect0/WAKEUP) was never connected.

Ok.

> > The kernel is probably not the place to be working around issues like
> > that,

> > even if serdev at least allows for such hacks to be fairly
> > isolated in drivers (unlike some of the earlier proposals touching core
> > code).
> 
> Please tell me why there are so many hacks for hardware issues in certain
> drivers. Any why those are good and this one (if it is one at all) is not.

Hacks are never good, but sometimes needed. But we should try to keep
them contained in drivers rather than allow them to spread to core code,
was what I was trying to say above.

> What I can learn from your discussion is that it might be considerable
> to add an optional gpio for the 0084 WAKEUP and add some logic to
> support users who have or will have that pin connected.
> 
> But even then we would need a driver to handle this gpio and issue
> an on/off impulse on the other to switch states. It would be a different
> driver (variant - maybe some CONFIG option or handled by code), but not
> "no driver".

Having a WAKEUP signal would allow for a more straight-forward
implementation, be it in the kernel or in user space.

> >> I see three reasonable options how this presentation can be done:
> >> 
> >> 1. char device
> >> 2. tty device
> >> 3. some new gps interface API (similar to network, bluetooth interfaces)
> >> 4. no driver and use the UART tty directly
> >> 
> >> Pros and cons:
> > 
> >> 4. no driver and use UART directly
> >> + a non-solution seems to be attractive
> >> - must turn on/off chip by gpio hacks from user-space
> > 
> > I'm not sure that would amount to more of hack then doing it in the
> > kernel would.
> 
> It might not be big effort in the user-space code/scripts.
> 
> But much effort to convince all the plethora of user-space client maintainers
> to integrate something. And have them roll out. And have distributions take 
> it.
> And have users upgrade to it. 5 years later...
> 
> Do you think it is easier to convince them than you? They usually assume a
> power management issue should be solved by the kernel driver.
> 
> That is what Andreas did remark as motivation: provide a solution
> for *existing* user spaces.

I understand that this is what you want, but that in itself is not a
sufficient reason to put something in the kernel.

> >> - can not guarantee (!) to power off the chip if the last user-space
> >> process using it is killed (which is essential for power-management of
> >> a handheld, battery operated device)
> > 
> > 

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-18 10:52, p...@codeaurora.org wrote:

On 2018-01-17 22:15, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+   driver = pci_find_dpc_service(udev);
+#endif
 #if IS_ENABLED(CONFIG_PCIEAER)
-   /* Use the aer driver of the component firstly */
-   driver = pci_find_aer_service(udev);


I think we need a pci_find_service function that unifies these two.


Right now, find_xxx_service are in their respective file and exporting 
it.

which makes sense no less than having generic function.

If I have to change pci_find_service(, int service_name) then it
has to be somewhere in generic file.
probably portdrv_core.c

either way I am fine but just thinking out if its really required.

Regards,
Oza.


Should I be doing in next patch-set series ?

Regards,
Oza.

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-18 10:52, p...@codeaurora.org wrote:

On 2018-01-17 22:15, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+   driver = pci_find_dpc_service(udev);
+#endif
 #if IS_ENABLED(CONFIG_PCIEAER)
-   /* Use the aer driver of the component firstly */
-   driver = pci_find_aer_service(udev);


I think we need a pci_find_service function that unifies these two.


Right now, find_xxx_service are in their respective file and exporting 
it.

which makes sense no less than having generic function.

If I have to change pci_find_service(, int service_name) then it
has to be somewhere in generic file.
probably portdrv_core.c

either way I am fine but just thinking out if its really required.

Regards,
Oza.


Should I be doing in next patch-set series ?

Regards,
Oza.

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-18 10:47, p...@codeaurora.org wrote:

On 2018-01-17 22:16, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+++ b/include/linux/dpc.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DPC_H_
+#define _DPC_H_
+
+#define DPC_FATAL  4
+
+#endif //_DPC_H_
+


can you keep this in drivers/pci.h and get rid of this file?


I thought about this, but if I keep it in drivers/pci.h,
then AER's defines have to be in that as well. (for unification)

and then all the dependent files who are using AER_FATAL such as
drivers/acpi/apei/ghees.c
have to go on including this drivers file which is odd way of doing it.

So I am not very sure about thissince AER_FATAL are in aer.h, I
have made dpc.h


Regards,
Oza.


Should I be doing in next patch-set series ?

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-18 10:47, p...@codeaurora.org wrote:

On 2018-01-17 22:16, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+++ b/include/linux/dpc.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DPC_H_
+#define _DPC_H_
+
+#define DPC_FATAL  4
+
+#endif //_DPC_H_
+


can you keep this in drivers/pci.h and get rid of this file?


I thought about this, but if I keep it in drivers/pci.h,
then AER's defines have to be in that as well. (for unification)

and then all the dependent files who are using AER_FATAL such as
drivers/acpi/apei/ghees.c
have to go on including this drivers file which is odd way of doing it.

So I am not very sure about thissince AER_FATAL are in aer.h, I
have made dpc.h


Regards,
Oza.


Should I be doing in next patch-set series ?

Re: [PATCH 2/3] power: supply: add cros-ec USB PD charger driver.

[Aishwarya Pant]

On Wed, Jan 17, 2018 at 06:59:31PM +0100, Enric Balletbo i Serra wrote:
> From: Sameer Nanda 
> 
> This driver gets various bits of information about what is connected to
> USB PD ports from the EC and converts that into power_supply properties.
> 
> Signed-off-by: Sameer Nanda 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>  drivers/power/supply/Kconfig  |  11 +
>  drivers/power/supply/Makefile |   1 +
>  drivers/power/supply/cros_usbpd-charger.c | 953 
> ++
>  include/linux/mfd/cros_ec.h   |   3 +
>  4 files changed, 968 insertions(+)
>  create mode 100644 drivers/power/supply/cros_usbpd-charger.c



> +static DEVICE_ATTR(ext_current_lim, 0664, get_ec_ext_current_lim,
> +set_ec_ext_current_lim);
> +static DEVICE_ATTR(ext_voltage_lim, 0664, get_ec_ext_voltage_lim,
> +set_ec_ext_voltage_lim);
> +

Hi

I see that you have added new files to the sysfs ABI. It would probably be a
good idea to have these new interfaces documented in Documentation/ABI.

Aishwarya

> +static struct attribute *__ext_power_cmds_attrs[] = {
> + _attr_ext_current_lim.attr,
> + _attr_ext_voltage_lim.attr,
> + NULL,
> +};

Re: [PATCH 2/3] power: supply: add cros-ec USB PD charger driver.

[Aishwarya Pant]

On Wed, Jan 17, 2018 at 06:59:31PM +0100, Enric Balletbo i Serra wrote:
> From: Sameer Nanda 
> 
> This driver gets various bits of information about what is connected to
> USB PD ports from the EC and converts that into power_supply properties.
> 
> Signed-off-by: Sameer Nanda 
> Signed-off-by: Enric Balletbo i Serra 
> ---
>  drivers/power/supply/Kconfig  |  11 +
>  drivers/power/supply/Makefile |   1 +
>  drivers/power/supply/cros_usbpd-charger.c | 953 
> ++
>  include/linux/mfd/cros_ec.h   |   3 +
>  4 files changed, 968 insertions(+)
>  create mode 100644 drivers/power/supply/cros_usbpd-charger.c



> +static DEVICE_ATTR(ext_current_lim, 0664, get_ec_ext_current_lim,
> +set_ec_ext_current_lim);
> +static DEVICE_ATTR(ext_voltage_lim, 0664, get_ec_ext_voltage_lim,
> +set_ec_ext_voltage_lim);
> +

Hi

I see that you have added new files to the sysfs ABI. It would probably be a
good idea to have these new interfaces documented in Documentation/ABI.

Aishwarya

> +static struct attribute *__ext_power_cmds_attrs[] = {
> + _attr_ext_current_lim.attr,
> + _attr_ext_voltage_lim.attr,
> + NULL,
> +};

[ANNOUNCE] Git v2.16.0

[Junio C Hamano]

The latest feature release Git v2.16.0 is now available at the
usual places.  It is comprised of 509 non-merge commits since
v2.15.0, contributed by 91 people, 26 of which are new faces.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.16.0'
tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.15.0 are as follows.
Welcome to the Git development community!

  Albert Astals Cid, Antoine Beaupré, Damien Marié, Daniel
  Bensoussan, Fangyi Zhou, Florian Klink, Gennady Kupava, Guillaume
  Castagnino, Haaris Mehmood, Hans Jerry Illikainen, Ingo Ruhnke,
  Jakub Bereżański, Jean Carlo Machado, Julien Dusser, J Wyman,
  Kevin, Louis Bettens, Łukasz Stelmach, Marius Paliga, Olga
  Telezhnaya, Rafael Ascensão, Robert Abel, Robert P. J. Day,
  Shuyu Wei, Wei Shuyu, and Zhou Fangyi.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Alexander Shopov,
  Alex Vandiver, Anders Kaseorg, Andrey Okoshkin, Ann T Ropea,
  Beat Bolli, Ben Peart, Brandon Williams, brian m. carlson, Carlos
  Martín Nieto, Changwoo Ryu, Charles Bailey, Christian Couder,
  Christopher Díaz Riveros, Dave Borowitz, Dennis Kaarsemaker,
  Derrick Stolee, Dimitriy Ryazantcev, Elijah Newren, Emily Xie,
  Eric Sunshine, Eric Wong, Heiko Voigt, Jacob Keller, Jameson
  Miller, Jean-Noel Avila, Jeff Hostetler, Jeff King, Jiang Xin,
  Johannes Schindelin, Jonathan Nieder, Jonathan Tan, Jordi Mas,
  Junio C Hamano, Kaartic Sivaraam, Kevin Daudt, Lars Schneider,
  Liam Beguin, Luke Diamand, Martin Ågren, Michael Haggerty,
  Nicolas Morey-Chaisemartin, Peter Krefting, Phil Hord, Phillip
  Wood, Pranit Bauva, Prathamesh Chavan, Ralf Thielow, Ramsay
  Jones, Randall S. Becker, Rasmus Villemoes, René Scharfe,
  Simon Ruderich, Stefan Beller, Steffen Prohaska, Stephan Beyer,
  SZEDER Gábor, Thomas Braun, Thomas Gummerer, Todd Zullinger,
  Torsten Bögershausen, Trần Ngọc Quân, and W. Trevor King.



Git 2.16 Release Notes
==

Backward compatibility notes and other notable changes.

 * Use of an empty string as a pathspec element that is used for
   'everything matches' is now an error.


Updates since v2.15
---

UI, Workflows & Features

 * An empty string as a pathspec element that means "everything"
   i.e. 'git add ""', is now illegal.  We started this by first
   deprecating and warning a pathspec that has such an element in
   2.11 (Nov 2016).

 * A hook script that is set unexecutable is simply ignored.  Git
   notifies when such a file is ignored, unless the message is
   squelched via advice.ignoredHook configuration.

 * "git pull" has been taught to accept "--[no-]signoff" option and
   pass it down to "git merge".

 * The "--push-option=" option to "git push" now defaults to a
   list of strings configured via push.pushOption variable.

 * "gitweb" checks if a directory is searchable with Perl's "-x"
   operator, which can be enhanced by using "filetest 'access'"
   pragma, which now we do.

 * "git stash save" has been deprecated in favour of "git stash push".

 * The set of paths output from "git status --ignored" was tied
   closely with its "--untracked=" option, but now it can be
   controlled more flexibly.  Most notably, a directory that is
   ignored because it is listed to be ignored in the ignore/exclude
   mechanism can be handled differently from a directory that ends up
   to be ignored only because all files in it are ignored.

 * The remote-helper for talking to MediaWiki has been updated to
   truncate an overlong pagename so that ".mw" suffix can still be
   added.

 * The remote-helper for talking to MediaWiki has been updated to
   work with mediawiki namespaces.

 * The "--format=..." option "git for-each-ref" takes learned to show
   the name of the 'remote' repository and the ref at the remote side
   that is affected for 'upstream' and 'push' via "%(push:remotename)"
   and friends.

 * Doc and message updates to teach users "bisect view" is a synonym
   for "bisect visualize".

 * "git bisect run" that did not specify any command to run used to go
   ahead and treated all commits to be tested as 'good'.  This has
   been corrected by making the command error out.

 * The SubmittingPatches document has been converted to produce an
   HTML version via AsciiDoc/Asciidoctor.

 * We learned to optionally talk to a file system monitor via new
   fsmonitor extension to speed up "git status" and other operations
   that need to see which paths have been modified.  Currently we only
   support "watchman".  See File System Monitor section of
   git-update-index(1) for more detail.

 * 

[ANNOUNCE] Git v2.16.0

[Junio C Hamano]

The latest feature release Git v2.16.0 is now available at the
usual places.  It is comprised of 509 non-merge commits since
v2.15.0, contributed by 91 people, 26 of which are new faces.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

The following public repositories all have a copy of the 'v2.16.0'
tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.15.0 are as follows.
Welcome to the Git development community!

  Albert Astals Cid, Antoine Beaupré, Damien Marié, Daniel
  Bensoussan, Fangyi Zhou, Florian Klink, Gennady Kupava, Guillaume
  Castagnino, Haaris Mehmood, Hans Jerry Illikainen, Ingo Ruhnke,
  Jakub Bereżański, Jean Carlo Machado, Julien Dusser, J Wyman,
  Kevin, Louis Bettens, Łukasz Stelmach, Marius Paliga, Olga
  Telezhnaya, Rafael Ascensão, Robert Abel, Robert P. J. Day,
  Shuyu Wei, Wei Shuyu, and Zhou Fangyi.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Alexander Shopov,
  Alex Vandiver, Anders Kaseorg, Andrey Okoshkin, Ann T Ropea,
  Beat Bolli, Ben Peart, Brandon Williams, brian m. carlson, Carlos
  Martín Nieto, Changwoo Ryu, Charles Bailey, Christian Couder,
  Christopher Díaz Riveros, Dave Borowitz, Dennis Kaarsemaker,
  Derrick Stolee, Dimitriy Ryazantcev, Elijah Newren, Emily Xie,
  Eric Sunshine, Eric Wong, Heiko Voigt, Jacob Keller, Jameson
  Miller, Jean-Noel Avila, Jeff Hostetler, Jeff King, Jiang Xin,
  Johannes Schindelin, Jonathan Nieder, Jonathan Tan, Jordi Mas,
  Junio C Hamano, Kaartic Sivaraam, Kevin Daudt, Lars Schneider,
  Liam Beguin, Luke Diamand, Martin Ågren, Michael Haggerty,
  Nicolas Morey-Chaisemartin, Peter Krefting, Phil Hord, Phillip
  Wood, Pranit Bauva, Prathamesh Chavan, Ralf Thielow, Ramsay
  Jones, Randall S. Becker, Rasmus Villemoes, René Scharfe,
  Simon Ruderich, Stefan Beller, Steffen Prohaska, Stephan Beyer,
  SZEDER Gábor, Thomas Braun, Thomas Gummerer, Todd Zullinger,
  Torsten Bögershausen, Trần Ngọc Quân, and W. Trevor King.



Git 2.16 Release Notes
==

Backward compatibility notes and other notable changes.

 * Use of an empty string as a pathspec element that is used for
   'everything matches' is now an error.


Updates since v2.15
---

UI, Workflows & Features

 * An empty string as a pathspec element that means "everything"
   i.e. 'git add ""', is now illegal.  We started this by first
   deprecating and warning a pathspec that has such an element in
   2.11 (Nov 2016).

 * A hook script that is set unexecutable is simply ignored.  Git
   notifies when such a file is ignored, unless the message is
   squelched via advice.ignoredHook configuration.

 * "git pull" has been taught to accept "--[no-]signoff" option and
   pass it down to "git merge".

 * The "--push-option=" option to "git push" now defaults to a
   list of strings configured via push.pushOption variable.

 * "gitweb" checks if a directory is searchable with Perl's "-x"
   operator, which can be enhanced by using "filetest 'access'"
   pragma, which now we do.

 * "git stash save" has been deprecated in favour of "git stash push".

 * The set of paths output from "git status --ignored" was tied
   closely with its "--untracked=" option, but now it can be
   controlled more flexibly.  Most notably, a directory that is
   ignored because it is listed to be ignored in the ignore/exclude
   mechanism can be handled differently from a directory that ends up
   to be ignored only because all files in it are ignored.

 * The remote-helper for talking to MediaWiki has been updated to
   truncate an overlong pagename so that ".mw" suffix can still be
   added.

 * The remote-helper for talking to MediaWiki has been updated to
   work with mediawiki namespaces.

 * The "--format=..." option "git for-each-ref" takes learned to show
   the name of the 'remote' repository and the ref at the remote side
   that is affected for 'upstream' and 'push' via "%(push:remotename)"
   and friends.

 * Doc and message updates to teach users "bisect view" is a synonym
   for "bisect visualize".

 * "git bisect run" that did not specify any command to run used to go
   ahead and treated all commits to be tested as 'good'.  This has
   been corrected by making the command error out.

 * The SubmittingPatches document has been converted to produce an
   HTML version via AsciiDoc/Asciidoctor.

 * We learned to optionally talk to a file system monitor via new
   fsmonitor extension to speed up "git status" and other operations
   that need to see which paths have been modified.  Currently we only
   support "watchman".  See File System Monitor section of
   git-update-index(1) for more detail.

 * 

Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Sergey Senozhatsky]

On (01/17/18 12:05), Tejun Heo wrote:
[..]
> > This could very well be a great place to force offloading. If a printk
> > is called from within a printk, at the same context (normal, softirq,
> > irq or NMI), then we should trigger the offloading.
> 
> I was thinking more of a timeout based approach (ie. if stuck for
> longer than X or X messages, offload)

yep, that's what I want. for a whole bunch of different reasons.

-ss

Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Sergey Senozhatsky]

On (01/17/18 12:05), Tejun Heo wrote:
[..]
> > This could very well be a great place to force offloading. If a printk
> > is called from within a printk, at the same context (normal, softirq,
> > irq or NMI), then we should trigger the offloading.
> 
> I was thinking more of a timeout based approach (ie. if stuck for
> longer than X or X messages, offload)

yep, that's what I want. for a whole bunch of different reasons.

-ss

Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Sergey Senozhatsky]

On (01/17/18 12:12), Steven Rostedt wrote:
[..]
>  /*
>   * Can we actually use the console at this time on this cpu?
> @@ -2333,6 +2390,7 @@ void console_unlock(void)
>  
>   for (;;) {
>   struct printk_log *msg;
> + bool offload;
>   size_t ext_len = 0;
>   size_t len;
>  
> @@ -2393,15 +2451,20 @@ void console_unlock(void)
>* waiter waiting to take over.
>*/
>   console_lock_spinning_enable();
> + offload = recursion_check_start();
>  
>   stop_critical_timings();/* don't trace print latency */
>   call_console_drivers(ext_text, ext_len, text, len);
>   start_critical_timings();
>  
> + recursion_check_finish(offload);
> +
>   if (console_lock_spinning_disable_and_check()) {
>   printk_safe_exit_irqrestore(flags);
>   return;
>   }
> + if (offload)
> + kick_offload_thread();
>  
>   printk_safe_exit_irqrestore(flags);


but we call console drivers in printk_safe.
printk -> console_drivers -> printk will be
redirected to this-CPU printk_safe buffer.

-ss

Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

[Sergey Senozhatsky]

On (01/17/18 12:12), Steven Rostedt wrote:
[..]
>  /*
>   * Can we actually use the console at this time on this cpu?
> @@ -2333,6 +2390,7 @@ void console_unlock(void)
>  
>   for (;;) {
>   struct printk_log *msg;
> + bool offload;
>   size_t ext_len = 0;
>   size_t len;
>  
> @@ -2393,15 +2451,20 @@ void console_unlock(void)
>* waiter waiting to take over.
>*/
>   console_lock_spinning_enable();
> + offload = recursion_check_start();
>  
>   stop_critical_timings();/* don't trace print latency */
>   call_console_drivers(ext_text, ext_len, text, len);
>   start_critical_timings();
>  
> + recursion_check_finish(offload);
> +
>   if (console_lock_spinning_disable_and_check()) {
>   printk_safe_exit_irqrestore(flags);
>   return;
>   }
> + if (offload)
> + kick_offload_thread();
>  
>   printk_safe_exit_irqrestore(flags);


but we call console drivers in printk_safe.
printk -> console_drivers -> printk will be
redirected to this-CPU printk_safe buffer.

-ss

[PATCH 2/4] i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly

[George Cherian]

In case of transaction with I2C_M_RECV_LEN set, make sure the driver reads
the first byte and then updates the RX fifo with the expected length. Set
threshold to 1 byte so that driver gets an interrupt on receiving the first 
byte.
After which the transfer length is updated depending on the received length.

Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 31 ---
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 6d78cdc..b5b224e 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -125,7 +125,16 @@ static void xlp9xx_i2c_update_rx_fifo_thres(struct 
xlp9xx_i2c_dev *priv)
 {
u32 thres;
 
-   thres = min(priv->msg_buf_remaining, XLP9XX_I2C_FIFO_SIZE);
+   if (priv->len_recv)
+   /* interrupt after the first read to examine
+* the length byte before proceeding further
+*/
+   thres = 1;
+   else if (priv->msg_buf_remaining > XLP9XX_I2C_FIFO_SIZE)
+   thres = XLP9XX_I2C_FIFO_SIZE;
+   else
+   thres = priv->msg_buf_remaining;
+
xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_MFIFOCTRL,
 thres << XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT);
 }
@@ -144,7 +153,7 @@ static void xlp9xx_i2c_fill_tx_fifo(struct xlp9xx_i2c_dev 
*priv)
 
 static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 {
-   u32 len, i;
+   u32 len, i, val;
u8 rlen, *buf = priv->msg_buf;
 
len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
@@ -156,19 +165,27 @@ static void xlp9xx_i2c_drain_rx_fifo(struct 
xlp9xx_i2c_dev *priv)
rlen = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
*buf++ = rlen;
len--;
+
if (priv->client_pec)
++rlen;
/* update remaining bytes and message length */
priv->msg_buf_remaining = rlen;
priv->msg_len = rlen + 1;
priv->len_recv = false;
-   }
 
-   len = min(priv->msg_buf_remaining, len);
-   for (i = 0; i < len; i++, buf++)
-   *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+   /* Update transfer length to read only actual data */
+   val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
+   val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
+   ((rlen + 1) << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
+   xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+   } else {
+   len = min(priv->msg_buf_remaining, len);
+   for (i = 0; i < len; i++, buf++)
+   *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+
+   priv->msg_buf_remaining -= len;
+   }
 
-   priv->msg_buf_remaining -= len;
priv->msg_buf = buf;
 
if (priv->msg_buf_remaining)
-- 
2.1.4

[PATCH 2/4] i2c: xlp9xx: Handle transactions with I2C_M_RECV_LEN properly

[George Cherian]

In case of transaction with I2C_M_RECV_LEN set, make sure the driver reads
the first byte and then updates the RX fifo with the expected length. Set
threshold to 1 byte so that driver gets an interrupt on receiving the first 
byte.
After which the transfer length is updated depending on the received length.

Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 31 ---
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 6d78cdc..b5b224e 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -125,7 +125,16 @@ static void xlp9xx_i2c_update_rx_fifo_thres(struct 
xlp9xx_i2c_dev *priv)
 {
u32 thres;
 
-   thres = min(priv->msg_buf_remaining, XLP9XX_I2C_FIFO_SIZE);
+   if (priv->len_recv)
+   /* interrupt after the first read to examine
+* the length byte before proceeding further
+*/
+   thres = 1;
+   else if (priv->msg_buf_remaining > XLP9XX_I2C_FIFO_SIZE)
+   thres = XLP9XX_I2C_FIFO_SIZE;
+   else
+   thres = priv->msg_buf_remaining;
+
xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_MFIFOCTRL,
 thres << XLP9XX_I2C_MFIFOCTRL_HITH_SHIFT);
 }
@@ -144,7 +153,7 @@ static void xlp9xx_i2c_fill_tx_fifo(struct xlp9xx_i2c_dev 
*priv)
 
 static void xlp9xx_i2c_drain_rx_fifo(struct xlp9xx_i2c_dev *priv)
 {
-   u32 len, i;
+   u32 len, i, val;
u8 rlen, *buf = priv->msg_buf;
 
len = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_FIFOWCNT) &
@@ -156,19 +165,27 @@ static void xlp9xx_i2c_drain_rx_fifo(struct 
xlp9xx_i2c_dev *priv)
rlen = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
*buf++ = rlen;
len--;
+
if (priv->client_pec)
++rlen;
/* update remaining bytes and message length */
priv->msg_buf_remaining = rlen;
priv->msg_len = rlen + 1;
priv->len_recv = false;
-   }
 
-   len = min(priv->msg_buf_remaining, len);
-   for (i = 0; i < len; i++, buf++)
-   *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+   /* Update transfer length to read only actual data */
+   val = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_CTRL);
+   val = (val & ~XLP9XX_I2C_CTRL_MCTLEN_MASK) |
+   ((rlen + 1) << XLP9XX_I2C_CTRL_MCTLEN_SHIFT);
+   xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CTRL, val);
+   } else {
+   len = min(priv->msg_buf_remaining, len);
+   for (i = 0; i < len; i++, buf++)
+   *buf = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_MRXFIFO);
+
+   priv->msg_buf_remaining -= len;
+   }
 
-   priv->msg_buf_remaining -= len;
priv->msg_buf = buf;
 
if (priv->msg_buf_remaining)
-- 
2.1.4

[PATCH 3/4] i2c: xlp9xx: report SMBus block read functionality

[George Cherian]

From: Dmitry Bazhenov 

Report SMBus block read functionality which is actually supported.

Signed-off-by: Dmitry Bazhenov 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index b5b224e..1f6d780 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -374,8 +374,8 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap, struct 
i2c_msg *msgs,
 
 static u32 xlp9xx_i2c_functionality(struct i2c_adapter *adapter)
 {
-   return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C |
-   I2C_FUNC_10BIT_ADDR;
+   return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+   I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR;
 }
 
 static const struct i2c_algorithm xlp9xx_i2c_algo = {
-- 
2.1.4

[PATCH 3/4] i2c: xlp9xx: report SMBus block read functionality

[George Cherian]

From: Dmitry Bazhenov 

Report SMBus block read functionality which is actually supported.

Signed-off-by: Dmitry Bazhenov 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index b5b224e..1f6d780 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -374,8 +374,8 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap, struct 
i2c_msg *msgs,
 
 static u32 xlp9xx_i2c_functionality(struct i2c_adapter *adapter)
 {
-   return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_I2C |
-   I2C_FUNC_10BIT_ADDR;
+   return I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_READ_BLOCK_DATA |
+   I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR;
 }
 
 static const struct i2c_algorithm xlp9xx_i2c_algo = {
-- 
2.1.4

[PATCH 1/4] i2c: xlp9xx: return ENXIO on slave address NACK

[George Cherian]

From: Dmitry Bazhenov 

Fix the driver violation of the common practice to return
ENXIO error on a slave address NACK.

Signed-off-by: Dmitry Bazhenov 
Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index b970bf8..6d78cdc 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -324,7 +324,8 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, 
struct i2c_msg *msg,
dev_dbg(priv->dev, "transfer error %x!\n", priv->msg_err);
if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR)
xlp9xx_i2c_init(priv);
-   return -EIO;
+   return (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) ?
+   -ENXIO : -EIO;
}
 
if (timeleft == 0) {
-- 
2.1.4

[PATCH 4/4] i2c: xlp9xx: Check for Bus state after every transfer

[George Cherian]

I2C bus enters the STOP condition after the DATA_DONE interrupt is raised.
Essentially the driver should be checking the bus state before sending
the next transaction. In case the next transaction is initiated while the
bus is busy, the prior transactions stop condition is not acheived.
Add the check to make sure the bus is not busy before next transaction.

Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 1f6d780..fa9d5ee 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define XLP9XX_I2C_DIV 0x0
 #define XLP9XX_I2C_CTRL0x1
@@ -36,6 +37,8 @@
 #define XLP9XX_I2C_TIMEOUT 0X10
 #define XLP9XX_I2C_GENCALLADDR 0x11
 
+#define XLP9XX_I2C_STATUS_BUSY BIT(0)
+
 #define XLP9XX_I2C_CMD_START   BIT(7)
 #define XLP9XX_I2C_CMD_STOPBIT(6)
 #define XLP9XX_I2C_CMD_READBIT(5)
@@ -71,6 +74,7 @@
 #define XLP9XX_I2C_HIGH_FREQ   40
 #define XLP9XX_I2C_FIFO_SIZE   0x80U
 #define XLP9XX_I2C_TIMEOUT_MS  1000
+#define XLP9XX_I2C_BUSY_TIMEOUT50
 
 #define XLP9XX_I2C_FIFO_WCNT_MASK  0xff
 #define XLP9XX_I2C_STATUS_ERRMASK  (XLP9XX_I2C_INTEN_ARLOST | \
@@ -264,7 +268,8 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, 
struct i2c_msg *msg,
   int last_msg)
 {
unsigned long timeleft;
-   u32 intr_mask, cmd, val, len;
+   u32 intr_mask, cmd, val, len, status;
+   u32 busy_timeout = XLP9XX_I2C_BUSY_TIMEOUT;
 
priv->msg_buf = msg->buf;
priv->msg_buf_remaining = priv->msg_len = msg->len;
@@ -351,6 +356,19 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev 
*priv, struct i2c_msg *msg,
return -ETIMEDOUT;
}
 
+   while (last_msg && busy_timeout) {
+   status = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_STATUS);
+   if ((status & XLP9XX_I2C_STATUS_BUSY) == 0)
+   break;
+
+   busy_timeout--;
+   udelay(1);
+   }
+
+   if (!busy_timeout) {
+   dev_dbg(priv->dev, "i2c bus busy for too long after 
transfer\n");
+   return -EIO;
+   }
/* update msg->len with actual received length */
if (msg->flags & I2C_M_RECV_LEN)
msg->len = priv->msg_len;
-- 
2.1.4

[PATCH 1/4] i2c: xlp9xx: return ENXIO on slave address NACK

[George Cherian]

From: Dmitry Bazhenov 

Fix the driver violation of the common practice to return
ENXIO error on a slave address NACK.

Signed-off-by: Dmitry Bazhenov 
Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index b970bf8..6d78cdc 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -324,7 +324,8 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, 
struct i2c_msg *msg,
dev_dbg(priv->dev, "transfer error %x!\n", priv->msg_err);
if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR)
xlp9xx_i2c_init(priv);
-   return -EIO;
+   return (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) ?
+   -ENXIO : -EIO;
}
 
if (timeleft == 0) {
-- 
2.1.4

[PATCH 4/4] i2c: xlp9xx: Check for Bus state after every transfer

[George Cherian]

I2C bus enters the STOP condition after the DATA_DONE interrupt is raised.
Essentially the driver should be checking the bus state before sending
the next transaction. In case the next transaction is initiated while the
bus is busy, the prior transactions stop condition is not acheived.
Add the check to make sure the bus is not busy before next transaction.

Signed-off-by: George Cherian 
---
 drivers/i2c/busses/i2c-xlp9xx.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 1f6d780..fa9d5ee 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define XLP9XX_I2C_DIV 0x0
 #define XLP9XX_I2C_CTRL0x1
@@ -36,6 +37,8 @@
 #define XLP9XX_I2C_TIMEOUT 0X10
 #define XLP9XX_I2C_GENCALLADDR 0x11
 
+#define XLP9XX_I2C_STATUS_BUSY BIT(0)
+
 #define XLP9XX_I2C_CMD_START   BIT(7)
 #define XLP9XX_I2C_CMD_STOPBIT(6)
 #define XLP9XX_I2C_CMD_READBIT(5)
@@ -71,6 +74,7 @@
 #define XLP9XX_I2C_HIGH_FREQ   40
 #define XLP9XX_I2C_FIFO_SIZE   0x80U
 #define XLP9XX_I2C_TIMEOUT_MS  1000
+#define XLP9XX_I2C_BUSY_TIMEOUT50
 
 #define XLP9XX_I2C_FIFO_WCNT_MASK  0xff
 #define XLP9XX_I2C_STATUS_ERRMASK  (XLP9XX_I2C_INTEN_ARLOST | \
@@ -264,7 +268,8 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, 
struct i2c_msg *msg,
   int last_msg)
 {
unsigned long timeleft;
-   u32 intr_mask, cmd, val, len;
+   u32 intr_mask, cmd, val, len, status;
+   u32 busy_timeout = XLP9XX_I2C_BUSY_TIMEOUT;
 
priv->msg_buf = msg->buf;
priv->msg_buf_remaining = priv->msg_len = msg->len;
@@ -351,6 +356,19 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev 
*priv, struct i2c_msg *msg,
return -ETIMEDOUT;
}
 
+   while (last_msg && busy_timeout) {
+   status = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_STATUS);
+   if ((status & XLP9XX_I2C_STATUS_BUSY) == 0)
+   break;
+
+   busy_timeout--;
+   udelay(1);
+   }
+
+   if (!busy_timeout) {
+   dev_dbg(priv->dev, "i2c bus busy for too long after 
transfer\n");
+   return -EIO;
+   }
/* update msg->len with actual received length */
if (msg->flags & I2C_M_RECV_LEN)
msg->len = priv->msg_len;
-- 
2.1.4

Re: [PATCH v5 4/4] PCI/DPC: Enumerate the devices after DPC trigger event

[poza]

On 2018-01-18 08:26, Keith Busch wrote:

On Wed, Jan 17, 2018 at 08:27:39AM -0800, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:
> +static bool dpc_wait_link_active(struct pci_dev *pdev)
> +{

I think you can also make this function common instead of making 
another copy here.

Of course, this would be another patch.


It is actually very similar to __pcie_wait_link_active in pciehp_hpc.c,
so there's some opprotunity to make even more common code.


in that case there has to be a generic function in
drives/pci.c

which addresses folowing functions from

pcie-dpc.c:
dpc_wait_link_inactive
dpc_wait_link_active

drivers/pci/hotplug/pciehp_hpc.c
pcie_wait_link_active


all aboe making one generic function to be moved to drives/pci.c

please let me know if this is okay.

Regards,
Oza.

Re: [PATCH v5 4/4] PCI/DPC: Enumerate the devices after DPC trigger event

[poza]

On 2018-01-18 08:26, Keith Busch wrote:

On Wed, Jan 17, 2018 at 08:27:39AM -0800, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:
> +static bool dpc_wait_link_active(struct pci_dev *pdev)
> +{

I think you can also make this function common instead of making 
another copy here.

Of course, this would be another patch.


It is actually very similar to __pcie_wait_link_active in pciehp_hpc.c,
so there's some opprotunity to make even more common code.


in that case there has to be a generic function in
drives/pci.c

which addresses folowing functions from

pcie-dpc.c:
dpc_wait_link_inactive
dpc_wait_link_active

drivers/pci/hotplug/pciehp_hpc.c
pcie_wait_link_active


all aboe making one generic function to be moved to drives/pci.c

please let me know if this is okay.

Regards,
Oza.

[PATCH v2] ASoC: use seq_file to dump the contents of dai_list,platform_list and codec_list

[Donglin Peng]

Now the debugfs files dais/platforms/codecs have a size limit PAGE_SIZE and
the user can not see the whole contents of dai_list/platform_list/codec_list
when they are larger than this limit.

This patch uses seq_file instead to make sure dais/platforms/codecs show the
full contents of dai_list/platform_list/codec_list.

Signed-off-by: Donglin Peng 
---
Changelog
v2
 -fix commit log error

 sound/soc/soc-core.c | 111 +--
 1 file changed, 37 insertions(+), 74 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c0edac80df34..7b582112e3fc 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -349,120 +349,84 @@ static void soc_init_codec_debugfs(struct 
snd_soc_component *component)
"ASoC: Failed to create codec register debugfs file\n");
 }
 
-static ssize_t codec_list_read_file(struct file *file, char __user *user_buf,
-   size_t count, loff_t *ppos)
+static int codec_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_codec *codec;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(codec, _list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-  codec->component.name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
+   list_for_each_entry(codec, _list, list)
+   seq_printf(m, "%s\n", codec->component.name);
 
mutex_unlock(_mutex);
 
-   if (ret >= 0)
-   ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
-
-   kfree(buf);
+   return 0;
+}
 
-   return ret;
+static int codec_list_seq_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, codec_list_seq_show, NULL);
 }
 
 static const struct file_operations codec_list_fops = {
-   .read = codec_list_read_file,
-   .llseek = default_llseek,/* read accesses f_pos */
+   .open = codec_list_seq_open,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
 };
 
-static ssize_t dai_list_read_file(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
+static int dai_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_component *component;
struct snd_soc_dai *dai;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(component, _list, list) {
-   list_for_each_entry(dai, >dai_list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-   dai->name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
-   }
+   list_for_each_entry(component, _list, list)
+   list_for_each_entry(dai, >dai_list, list)
+   seq_printf(m, "%s\n", dai->name);
 
mutex_unlock(_mutex);
 
-   ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
-
-   kfree(buf);
+   return 0;
+}
 
-   return ret;
+static int dai_list_seq_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, dai_list_seq_show, NULL);
 }
 
 static const struct file_operations dai_list_fops = {
-   .read = dai_list_read_file,
-   .llseek = default_llseek,/* read accesses f_pos */
+   .open = dai_list_seq_open,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
 };
 
-static ssize_t platform_list_read_file(struct file *file,
-  char __user *user_buf,
-  size_t count, loff_t *ppos)
+static int platform_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_platform *platform;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(platform, _list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-  platform->component.name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
+   list_for_each_entry(platform, 

[PATCH v2] ASoC: use seq_file to dump the contents of dai_list,platform_list and codec_list

[Donglin Peng]

Now the debugfs files dais/platforms/codecs have a size limit PAGE_SIZE and
the user can not see the whole contents of dai_list/platform_list/codec_list
when they are larger than this limit.

This patch uses seq_file instead to make sure dais/platforms/codecs show the
full contents of dai_list/platform_list/codec_list.

Signed-off-by: Donglin Peng 
---
Changelog
v2
 -fix commit log error

 sound/soc/soc-core.c | 111 +--
 1 file changed, 37 insertions(+), 74 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index c0edac80df34..7b582112e3fc 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -349,120 +349,84 @@ static void soc_init_codec_debugfs(struct 
snd_soc_component *component)
"ASoC: Failed to create codec register debugfs file\n");
 }
 
-static ssize_t codec_list_read_file(struct file *file, char __user *user_buf,
-   size_t count, loff_t *ppos)
+static int codec_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_codec *codec;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(codec, _list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-  codec->component.name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
+   list_for_each_entry(codec, _list, list)
+   seq_printf(m, "%s\n", codec->component.name);
 
mutex_unlock(_mutex);
 
-   if (ret >= 0)
-   ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
-
-   kfree(buf);
+   return 0;
+}
 
-   return ret;
+static int codec_list_seq_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, codec_list_seq_show, NULL);
 }
 
 static const struct file_operations codec_list_fops = {
-   .read = codec_list_read_file,
-   .llseek = default_llseek,/* read accesses f_pos */
+   .open = codec_list_seq_open,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
 };
 
-static ssize_t dai_list_read_file(struct file *file, char __user *user_buf,
- size_t count, loff_t *ppos)
+static int dai_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_component *component;
struct snd_soc_dai *dai;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(component, _list, list) {
-   list_for_each_entry(dai, >dai_list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-   dai->name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
-   }
+   list_for_each_entry(component, _list, list)
+   list_for_each_entry(dai, >dai_list, list)
+   seq_printf(m, "%s\n", dai->name);
 
mutex_unlock(_mutex);
 
-   ret = simple_read_from_buffer(user_buf, count, ppos, buf, ret);
-
-   kfree(buf);
+   return 0;
+}
 
-   return ret;
+static int dai_list_seq_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, dai_list_seq_show, NULL);
 }
 
 static const struct file_operations dai_list_fops = {
-   .read = dai_list_read_file,
-   .llseek = default_llseek,/* read accesses f_pos */
+   .open = dai_list_seq_open,
+   .read = seq_read,
+   .llseek = seq_lseek,
+   .release = single_release,
 };
 
-static ssize_t platform_list_read_file(struct file *file,
-  char __user *user_buf,
-  size_t count, loff_t *ppos)
+static int platform_list_seq_show(struct seq_file *m, void *v)
 {
-   char *buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-   ssize_t len, ret = 0;
struct snd_soc_platform *platform;
 
-   if (!buf)
-   return -ENOMEM;
-
mutex_lock(_mutex);
 
-   list_for_each_entry(platform, _list, list) {
-   len = snprintf(buf + ret, PAGE_SIZE - ret, "%s\n",
-  platform->component.name);
-   if (len >= 0)
-   ret += len;
-   if (ret > PAGE_SIZE) {
-   ret = PAGE_SIZE;
-   break;
-   }
-   }
+   list_for_each_entry(platform, _list, list)
+   

[PATCH v2] selftest: support run subset of selftests when running installed selftests

[Zong Li]

Simply use the first argument to specify the subset of selftests.
Use comma notation to separate multiple tests.
(e.g. ./run_kselftest.sh size,timers,...)
Default behaviour is running all selftests installed.

Each selftest be defined as function that we can run one of selftests.
The function name has underline as prefix to avoid confilct with
built-in command of shell.
(e.g. The exec is built-in command of shell)

Replace the hyphen with underline for cpu-hotplug and memory-hotplug.
Not all shell can use hyphen in function name, like sh, ash and so on.

Signed-off-by: Zong Li 
---
 tools/testing/selftests/Makefile| 17 +
 .../selftests/{cpu-hotplug => cpu_hotplug}/Makefile |  2 +-
 .../selftests/{cpu-hotplug => cpu_hotplug}/config   |  0
 .../{cpu-hotplug => cpu_hotplug}/cpu-on-off-test.sh |  0
 .../{memory-hotplug => memory_hotplug}/Makefile |  4 ++--
 .../selftests/{memory-hotplug => memory_hotplug}/config |  0
 .../mem-on-off-test.sh  |  0
 7 files changed, 16 insertions(+), 7 deletions(-)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/Makefile (66%)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/config (100%)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/cpu-on-off-test.sh 
(100%)
 rename tools/testing/selftests/{memory-hotplug => memory_hotplug}/Makefile 
(55%)
 rename tools/testing/selftests/{memory-hotplug => memory_hotplug}/config (100%)
 rename tools/testing/selftests/{memory-hotplug => 
memory_hotplug}/mem-on-off-test.sh (100%)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index eaf599d..dc9bb6c 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -4,7 +4,7 @@ TARGETS += bpf
 TARGETS += breakpoints
 TARGETS += capabilities
 TARGETS += cpufreq
-TARGETS += cpu-hotplug
+TARGETS += cpu_hotplug
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += firmware
@@ -17,7 +17,7 @@ TARGETS += kcmp
 TARGETS += lib
 TARGETS += membarrier
 TARGETS += memfd
-TARGETS += memory-hotplug
+TARGETS += memory_hotplug
 TARGETS += mount
 TARGETS += mqueue
 TARGETS += net
@@ -43,8 +43,8 @@ TARGETS += zram
 # Run "make quicktest=1 run_tests" or
 # "make quicktest=1 kselftest" from top level Makefile
 
-TARGETS_HOTPLUG = cpu-hotplug
-TARGETS_HOTPLUG += memory-hotplug
+TARGETS_HOTPLUG = cpu_hotplug
+TARGETS_HOTPLUG += memory_hotplug
 
 # Clear LDFLAGS and MAKEFLAGS if called from main
 # Makefile to avoid test build failures when test
@@ -121,13 +121,22 @@ ifdef INSTALL_PATH
 
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET;  \
+   echo "_$$TARGET()" >> $(ALL_SCRIPT); \
+   echo "{" >> $(ALL_SCRIPT); \
echo "echo ; echo Running tests in $$TARGET" >> $(ALL_SCRIPT); \
echo "echo " >> 
$(ALL_SCRIPT); \
echo "cd $$TARGET" >> $(ALL_SCRIPT); \
make -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET 
emit_tests >> $(ALL_SCRIPT); \
echo "cd \$$ROOT" >> $(ALL_SCRIPT); \
+   echo "}" >> $(ALL_SCRIPT); \
+   echo "" >> $(ALL_SCRIPT); \
done;
 
+   echo "TARGETS=\$${1:-\`ls -d */\`}" >> $(ALL_SCRIPT); \
+   echo "for TARGET in \$${TARGETS//,/ }; do" >> $(ALL_SCRIPT); \
+   echo "_\$${TARGET%/}" >> $(ALL_SCRIPT); \
+   echo "done" >> $(ALL_SCRIPT); \
+
chmod u+x $(ALL_SCRIPT)
 else
$(error Error: set INSTALL_PATH to use install)
diff --git a/tools/testing/selftests/cpu-hotplug/Makefile 
b/tools/testing/selftests/cpu_hotplug/Makefile
similarity index 66%
rename from tools/testing/selftests/cpu-hotplug/Makefile
rename to tools/testing/selftests/cpu_hotplug/Makefile
index d8be047..a9c88b9 100644
--- a/tools/testing/selftests/cpu-hotplug/Makefile
+++ b/tools/testing/selftests/cpu_hotplug/Makefile
@@ -6,6 +6,6 @@ TEST_PROGS := cpu-on-off-test.sh
 include ../lib.mk
 
 run_full_test:
-   @/bin/bash ./cpu-on-off-test.sh -a || echo "cpu-hotplug selftests: 
[FAIL]"
+   @/bin/bash ./cpu-on-off-test.sh -a || echo "cpu_hotplug selftests: 
[FAIL]"
 
 clean:
diff --git a/tools/testing/selftests/cpu-hotplug/config 
b/tools/testing/selftests/cpu_hotplug/config
similarity index 100%
rename from tools/testing/selftests/cpu-hotplug/config
rename to tools/testing/selftests/cpu_hotplug/config
diff --git a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh 
b/tools/testing/selftests/cpu_hotplug/cpu-on-off-test.sh
similarity index 100%
rename from tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh
rename to tools/testing/selftests/cpu_hotplug/cpu-on-off-test.sh
diff --git a/tools/testing/selftests/memory-hotplug/Makefile 
b/tools/testing/selftests/memory_hotplug/Makefile
similarity index 55%
rename from tools/testing/selftests/memory-hotplug/Makefile
rename to 

[PATCH v2] selftest: support run subset of selftests when running installed selftests

[Zong Li]

Simply use the first argument to specify the subset of selftests.
Use comma notation to separate multiple tests.
(e.g. ./run_kselftest.sh size,timers,...)
Default behaviour is running all selftests installed.

Each selftest be defined as function that we can run one of selftests.
The function name has underline as prefix to avoid confilct with
built-in command of shell.
(e.g. The exec is built-in command of shell)

Replace the hyphen with underline for cpu-hotplug and memory-hotplug.
Not all shell can use hyphen in function name, like sh, ash and so on.

Signed-off-by: Zong Li 
---
 tools/testing/selftests/Makefile| 17 +
 .../selftests/{cpu-hotplug => cpu_hotplug}/Makefile |  2 +-
 .../selftests/{cpu-hotplug => cpu_hotplug}/config   |  0
 .../{cpu-hotplug => cpu_hotplug}/cpu-on-off-test.sh |  0
 .../{memory-hotplug => memory_hotplug}/Makefile |  4 ++--
 .../selftests/{memory-hotplug => memory_hotplug}/config |  0
 .../mem-on-off-test.sh  |  0
 7 files changed, 16 insertions(+), 7 deletions(-)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/Makefile (66%)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/config (100%)
 rename tools/testing/selftests/{cpu-hotplug => cpu_hotplug}/cpu-on-off-test.sh 
(100%)
 rename tools/testing/selftests/{memory-hotplug => memory_hotplug}/Makefile 
(55%)
 rename tools/testing/selftests/{memory-hotplug => memory_hotplug}/config (100%)
 rename tools/testing/selftests/{memory-hotplug => 
memory_hotplug}/mem-on-off-test.sh (100%)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index eaf599d..dc9bb6c 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -4,7 +4,7 @@ TARGETS += bpf
 TARGETS += breakpoints
 TARGETS += capabilities
 TARGETS += cpufreq
-TARGETS += cpu-hotplug
+TARGETS += cpu_hotplug
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += firmware
@@ -17,7 +17,7 @@ TARGETS += kcmp
 TARGETS += lib
 TARGETS += membarrier
 TARGETS += memfd
-TARGETS += memory-hotplug
+TARGETS += memory_hotplug
 TARGETS += mount
 TARGETS += mqueue
 TARGETS += net
@@ -43,8 +43,8 @@ TARGETS += zram
 # Run "make quicktest=1 run_tests" or
 # "make quicktest=1 kselftest" from top level Makefile
 
-TARGETS_HOTPLUG = cpu-hotplug
-TARGETS_HOTPLUG += memory-hotplug
+TARGETS_HOTPLUG = cpu_hotplug
+TARGETS_HOTPLUG += memory_hotplug
 
 # Clear LDFLAGS and MAKEFLAGS if called from main
 # Makefile to avoid test build failures when test
@@ -121,13 +121,22 @@ ifdef INSTALL_PATH
 
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET;  \
+   echo "_$$TARGET()" >> $(ALL_SCRIPT); \
+   echo "{" >> $(ALL_SCRIPT); \
echo "echo ; echo Running tests in $$TARGET" >> $(ALL_SCRIPT); \
echo "echo " >> 
$(ALL_SCRIPT); \
echo "cd $$TARGET" >> $(ALL_SCRIPT); \
make -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET 
emit_tests >> $(ALL_SCRIPT); \
echo "cd \$$ROOT" >> $(ALL_SCRIPT); \
+   echo "}" >> $(ALL_SCRIPT); \
+   echo "" >> $(ALL_SCRIPT); \
done;
 
+   echo "TARGETS=\$${1:-\`ls -d */\`}" >> $(ALL_SCRIPT); \
+   echo "for TARGET in \$${TARGETS//,/ }; do" >> $(ALL_SCRIPT); \
+   echo "_\$${TARGET%/}" >> $(ALL_SCRIPT); \
+   echo "done" >> $(ALL_SCRIPT); \
+
chmod u+x $(ALL_SCRIPT)
 else
$(error Error: set INSTALL_PATH to use install)
diff --git a/tools/testing/selftests/cpu-hotplug/Makefile 
b/tools/testing/selftests/cpu_hotplug/Makefile
similarity index 66%
rename from tools/testing/selftests/cpu-hotplug/Makefile
rename to tools/testing/selftests/cpu_hotplug/Makefile
index d8be047..a9c88b9 100644
--- a/tools/testing/selftests/cpu-hotplug/Makefile
+++ b/tools/testing/selftests/cpu_hotplug/Makefile
@@ -6,6 +6,6 @@ TEST_PROGS := cpu-on-off-test.sh
 include ../lib.mk
 
 run_full_test:
-   @/bin/bash ./cpu-on-off-test.sh -a || echo "cpu-hotplug selftests: 
[FAIL]"
+   @/bin/bash ./cpu-on-off-test.sh -a || echo "cpu_hotplug selftests: 
[FAIL]"
 
 clean:
diff --git a/tools/testing/selftests/cpu-hotplug/config 
b/tools/testing/selftests/cpu_hotplug/config
similarity index 100%
rename from tools/testing/selftests/cpu-hotplug/config
rename to tools/testing/selftests/cpu_hotplug/config
diff --git a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh 
b/tools/testing/selftests/cpu_hotplug/cpu-on-off-test.sh
similarity index 100%
rename from tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh
rename to tools/testing/selftests/cpu_hotplug/cpu-on-off-test.sh
diff --git a/tools/testing/selftests/memory-hotplug/Makefile 
b/tools/testing/selftests/memory_hotplug/Makefile
similarity index 55%
rename from tools/testing/selftests/memory-hotplug/Makefile
rename to 

[PATCH v2] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

[Ganapatrao Kulkarni]

This erratum is observed on the ThunderX2 GICv3 ITS. When a
MOVI command is used to change affinity of a LPI to a collection/cpu
on another node, the LPI is not delivered to the cpu.
An additional INV command is required after the MOVI to deliver
the LPI to the new destination.

If we add INV after MOVI, there is a chance that we lose LPIs which
are raised when the affinity is changed. So for now, adding workaround fix
to disable inter node affinity change.

Signed-off-by: Ganapatrao Kulkarni 
---

v2: Added workaround to avoid inter node affinity change.

v1: Initial patch

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 10 ++
 drivers/irqchip/irq-gic-v3-its.c   | 21 -
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index fc1c884..fb27cb5 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174  
|
 | Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 | Cavium | ThunderX2 SMMUv3| #126| N/A 
|
 || | | 
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c9a7e9e..0dbf3bd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,16 @@ config ARM64_ERRATUM_843419
 
  If unsure, say Y.
 
+config CAVIUM_ERRATUM_174
+   bool "Cavium ThunderX2 erratum 174"
+   default y
+   help
+ Cavium ThunderX2 dual socket systems may loose interrupts
+ on affinity change to a cpu on other node.
+ This workaround fix avoids inter node affinity change.
+
+ If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 06f025f..b0cb528 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_CAVIUM_174(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -1102,7 +1103,8 @@ static int its_set_affinity(struct irq_data *d, const 
struct cpumask *mask_val,
return -EINVAL;
 
/* lpi cannot be routed to a redistributor that is on a foreign node */
-   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
+   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144 ||
+   its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) {
if (its_dev->its->numa_node >= 0) {
cpu_mask = cpumask_of_node(its_dev->its->numa_node);
if (!cpumask_intersects(mask_val, cpu_mask))
@@ -2904,6 +2906,15 @@ static int its_force_quiescent(void __iomem *base)
}
 }
 
+static bool __maybe_unused its_enable_quirk_cavium_174(void *data)
+{
+   struct its_node *its = data;
+
+   its->flags |= ITS_FLAGS_WORKAROUND_CAVIUM_174;
+
+   return true;
+}
+
 static bool __maybe_unused its_enable_quirk_cavium_22375(void *data)
 {
struct its_node *its = data;
@@ -3031,6 +3042,14 @@ static const struct gic_quirk its_quirks[] = {
.init   = its_enable_quirk_hip07_161600802,
},
 #endif
+#ifdef CONFIG_CAVIUM_ERRATUM_174
+   {
+   .desc   = "ITS: Cavium ThunderX2 erratum 174",
+   .iidr   = 0x13f,/* ThunderX2 pass A1/A2/B0 */
+   .mask   = 0x,
+   .init   = its_enable_quirk_cavium_174,
+   },
+#endif
{
}
 };
-- 
2.9.4

[PATCH v2] irqchip/gic-v3-its: Add workaround for ThunderX2 erratum #174

[Ganapatrao Kulkarni]

This erratum is observed on the ThunderX2 GICv3 ITS. When a
MOVI command is used to change affinity of a LPI to a collection/cpu
on another node, the LPI is not delivered to the cpu.
An additional INV command is required after the MOVI to deliver
the LPI to the new destination.

If we add INV after MOVI, there is a chance that we lose LPIs which
are raised when the affinity is changed. So for now, adding workaround fix
to disable inter node affinity change.

Signed-off-by: Ganapatrao Kulkarni 
---

v2: Added workaround to avoid inter node affinity change.

v1: Initial patch

 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 10 ++
 drivers/irqchip/irq-gic-v3-its.c   | 21 -
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index fc1c884..fb27cb5 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -63,6 +63,7 @@ stable kernels.
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX Core   | #30115  | CAVIUM_ERRATUM_30115
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 ITS   | #174| CAVIUM_ERRATUM_174  
|
 | Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 | Cavium | ThunderX2 SMMUv3| #126| N/A 
|
 || | | 
|
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index c9a7e9e..0dbf3bd 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,16 @@ config ARM64_ERRATUM_843419
 
  If unsure, say Y.
 
+config CAVIUM_ERRATUM_174
+   bool "Cavium ThunderX2 erratum 174"
+   default y
+   help
+ Cavium ThunderX2 dual socket systems may loose interrupts
+ on affinity change to a cpu on other node.
+ This workaround fix avoids inter node affinity change.
+
+ If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
bool "Cavium erratum 22375, 24313"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 06f025f..b0cb528 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -46,6 +46,7 @@
 #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING  (1ULL << 0)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_22375  (1ULL << 1)
 #define ITS_FLAGS_WORKAROUND_CAVIUM_23144  (1ULL << 2)
+#define ITS_FLAGS_WORKAROUND_CAVIUM_174(1ULL << 3)
 
 #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING(1 << 0)
 
@@ -1102,7 +1103,8 @@ static int its_set_affinity(struct irq_data *d, const 
struct cpumask *mask_val,
return -EINVAL;
 
/* lpi cannot be routed to a redistributor that is on a foreign node */
-   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144) {
+   if (its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_23144 ||
+   its_dev->its->flags & ITS_FLAGS_WORKAROUND_CAVIUM_174) {
if (its_dev->its->numa_node >= 0) {
cpu_mask = cpumask_of_node(its_dev->its->numa_node);
if (!cpumask_intersects(mask_val, cpu_mask))
@@ -2904,6 +2906,15 @@ static int its_force_quiescent(void __iomem *base)
}
 }
 
+static bool __maybe_unused its_enable_quirk_cavium_174(void *data)
+{
+   struct its_node *its = data;
+
+   its->flags |= ITS_FLAGS_WORKAROUND_CAVIUM_174;
+
+   return true;
+}
+
 static bool __maybe_unused its_enable_quirk_cavium_22375(void *data)
 {
struct its_node *its = data;
@@ -3031,6 +3042,14 @@ static const struct gic_quirk its_quirks[] = {
.init   = its_enable_quirk_hip07_161600802,
},
 #endif
+#ifdef CONFIG_CAVIUM_ERRATUM_174
+   {
+   .desc   = "ITS: Cavium ThunderX2 erratum 174",
+   .iidr   = 0x13f,/* ThunderX2 pass A1/A2/B0 */
+   .mask   = 0x,
+   .init   = its_enable_quirk_cavium_174,
+   },
+#endif
{
}
 };
-- 
2.9.4

Re: [PATCH v5 4/4] PCI/DPC: Enumerate the devices after DPC trigger event

[poza]

On 2018-01-17 21:57, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+static bool dpc_wait_link_active(struct pci_dev *pdev)
+{


I think you can also make this function common instead of making
another copy here.
Of course, this would be another patch.


ok I will make a separate patch taking one more parameter
dpc_wait_link_active(struct pci_dev *, bool)

if not in this series, then immediate one.

Regards,
Oza.

Re: [PATCH v5 4/4] PCI/DPC: Enumerate the devices after DPC trigger event

[poza]

On 2018-01-17 21:57, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+static bool dpc_wait_link_active(struct pci_dev *pdev)
+{


I think you can also make this function common instead of making
another copy here.
Of course, this would be another patch.


ok I will make a separate patch taking one more parameter
dpc_wait_link_active(struct pci_dev *, bool)

if not in this series, then immediate one.

Regards,
Oza.

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-17 22:15, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+   driver = pci_find_dpc_service(udev);
+#endif
 #if IS_ENABLED(CONFIG_PCIEAER)
-   /* Use the aer driver of the component firstly */
-   driver = pci_find_aer_service(udev);


I think we need a pci_find_service function that unifies these two.


Right now, find_xxx_service are in their respective file and exporting 
it.

which makes sense no less than having generic function.

If I have to change pci_find_service(, int service_name) then it has 
to be somewhere in generic file.

probably portdrv_core.c

either way I am fine but just thinking out if its really required.

Regards,
Oza.

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-17 22:15, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+   driver = pci_find_dpc_service(udev);
+#endif
 #if IS_ENABLED(CONFIG_PCIEAER)
-   /* Use the aer driver of the component firstly */
-   driver = pci_find_aer_service(udev);


I think we need a pci_find_service function that unifies these two.


Right now, find_xxx_service are in their respective file and exporting 
it.

which makes sense no less than having generic function.

If I have to change pci_find_service(, int service_name) then it has 
to be somewhere in generic file.

probably portdrv_core.c

either way I am fine but just thinking out if its really required.

Regards,
Oza.

Re: [PATCH] gitignore: add *.gcda files

[Jaejoong Kim]

2018-01-17 22:07 GMT+09:00 Peter Oberparleiter :
> On 09.01.2018 06:17, Jaejoong Kim wrote:
>> 2017-12-20 16:09 GMT+09:00 Jaejoong Kim :
>>> Ignore the *.gcda files generated by gcov
>>>
>>> Signed-off-by: Jaejoong Kim 
>>> ---
>>>  .gitignore | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 0c39aa2..580ef7c 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -39,6 +39,7 @@ Module.symvers
>>>  *.dwo
>>>  *.su
>>>  *.c.[012]*.*
>>> +*.gcda
>
> The gcov-kernel mechanism generates .gcda files for kernel code only as
> virtual files in debugfs, and not in the kernel source tree.

You are right. The .gcda files is from debugfs and I copied it to the
kernel source tree.

Sorry for the bother you. Please ignore this patch file.

Thanks
jaejoong

What source
> of .gcda files would be covered by this .gitignore change?
>
>>>
>>>  #
>>>  # Top-level generic files
>>> --
>>> 2.7.4
>>>
>
> --
> Peter Oberparleiter
> Linux on z Systems Development - IBM Germany
>

Re: [PATCH] gitignore: add *.gcda files

[Jaejoong Kim]

2018-01-17 22:07 GMT+09:00 Peter Oberparleiter :
> On 09.01.2018 06:17, Jaejoong Kim wrote:
>> 2017-12-20 16:09 GMT+09:00 Jaejoong Kim :
>>> Ignore the *.gcda files generated by gcov
>>>
>>> Signed-off-by: Jaejoong Kim 
>>> ---
>>>  .gitignore | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/.gitignore b/.gitignore
>>> index 0c39aa2..580ef7c 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -39,6 +39,7 @@ Module.symvers
>>>  *.dwo
>>>  *.su
>>>  *.c.[012]*.*
>>> +*.gcda
>
> The gcov-kernel mechanism generates .gcda files for kernel code only as
> virtual files in debugfs, and not in the kernel source tree.

You are right. The .gcda files is from debugfs and I copied it to the
kernel source tree.

Sorry for the bother you. Please ignore this patch file.

Thanks
jaejoong

What source
> of .gcda files would be covered by this .gitignore change?
>
>>>
>>>  #
>>>  # Top-level generic files
>>> --
>>> 2.7.4
>>>
>
> --
> Peter Oberparleiter
> Linux on z Systems Development - IBM Germany
>

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-17 22:16, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+++ b/include/linux/dpc.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DPC_H_
+#define _DPC_H_
+
+#define DPC_FATAL  4
+
+#endif //_DPC_H_
+


can you keep this in drivers/pci.h and get rid of this file?


I thought about this, but if I keep it in drivers/pci.h,
then AER's defines have to be in that as well. (for unification)

and then all the dependent files who are using AER_FATAL such as 
drivers/acpi/apei/ghees.c

have to go on including this drivers file which is odd way of doing it.

So I am not very sure about thissince AER_FATAL are in aer.h, I have 
made dpc.h



Regards,
Oza.

Re: [PATCH v5 3/4] PCI/DPC: Unify and plumb error handling into DPC

[poza]

On 2018-01-17 22:16, Sinan Kaya wrote:

On 1/17/2018 5:37 AM, Oza Pawandeep wrote:

+++ b/include/linux/dpc.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DPC_H_
+#define _DPC_H_
+
+#define DPC_FATAL  4
+
+#endif //_DPC_H_
+


can you keep this in drivers/pci.h and get rid of this file?


I thought about this, but if I keep it in drivers/pci.h,
then AER's defines have to be in that as well. (for unification)

and then all the dependent files who are using AER_FATAL such as 
drivers/acpi/apei/ghees.c

have to go on including this drivers file which is odd way of doing it.

So I am not very sure about thissince AER_FATAL are in aer.h, I have 
made dpc.h



Regards,
Oza.

Re: kexec reboot fails with extra wbinvd introduced for AME SME

[Tom Lendacky]

On 1/17/2018 8:29 PM, Dave Young wrote:
> On 01/17/18 at 06:14pm, Linus Torvalds wrote:
>> On Wed, Jan 17, 2018 at 5:47 PM, Dave Young  wrote:
>>>
>>> It does not work with just once wbinvd(), and it only works with
>>> removing the wbinvd() for me.  Tom's new post works for me as well
>>> since my cpu is an Intel i5-4200U.
>>
>> Intriguing.
>>
>> It's not like the wbinvd really should be that much of a deal.
>>
>> I think Tom's patch is fine and should be applied, but it does worry
>> me a bit that even a single wbinvd makes that much of a difference for
>> you. There is very little logical reason I can think of that a wbinvd
>> should make any difference what-so-ever on an i5-4200U.
>>
>> I wonder if you have some system issues, and wbinvd just happens to
>> trigger them. But I think we do wbinvd before a suspend-to-RAM too
>> (it's "ACPI_FLUSH_CPU_CACHE()" in the ACPI code). And the dmr code
>> dioes "wbinvd_on_all_cpus()" which does a cross-call etc.
>>
>> Would you mind experimenting a bit with that wbinvd?
>>
>> In particular, what happens if you enable it (so it's not hidden by
>> the SME check), but you move it up to before interrupts are disabled?
> 
> Will play with it more.  Actually I found the hang seems happens
> in code of arch/x86/kernel/relocate_kernel_64.S, there is another
> wbinvd there as well.

The wbinvd in arch/x86/kernel/relocate_kernel_64.S is only performed if
SME is active, so that one won't be executed on an Intel chip.

Thanks,
Tom

> 
>>
>> I'm wondering if there is some issue with MCE generation and wbinvd
>> and whatever, and doing it when the CPU is down and interrupts are
>> disabled causes some system issue..
>>
>> Does anybody have any other ideas?
>>
>> Linus

Re: kexec reboot fails with extra wbinvd introduced for AME SME

[Tom Lendacky]

On 1/17/2018 8:29 PM, Dave Young wrote:
> On 01/17/18 at 06:14pm, Linus Torvalds wrote:
>> On Wed, Jan 17, 2018 at 5:47 PM, Dave Young  wrote:
>>>
>>> It does not work with just once wbinvd(), and it only works with
>>> removing the wbinvd() for me.  Tom's new post works for me as well
>>> since my cpu is an Intel i5-4200U.
>>
>> Intriguing.
>>
>> It's not like the wbinvd really should be that much of a deal.
>>
>> I think Tom's patch is fine and should be applied, but it does worry
>> me a bit that even a single wbinvd makes that much of a difference for
>> you. There is very little logical reason I can think of that a wbinvd
>> should make any difference what-so-ever on an i5-4200U.
>>
>> I wonder if you have some system issues, and wbinvd just happens to
>> trigger them. But I think we do wbinvd before a suspend-to-RAM too
>> (it's "ACPI_FLUSH_CPU_CACHE()" in the ACPI code). And the dmr code
>> dioes "wbinvd_on_all_cpus()" which does a cross-call etc.
>>
>> Would you mind experimenting a bit with that wbinvd?
>>
>> In particular, what happens if you enable it (so it's not hidden by
>> the SME check), but you move it up to before interrupts are disabled?
> 
> Will play with it more.  Actually I found the hang seems happens
> in code of arch/x86/kernel/relocate_kernel_64.S, there is another
> wbinvd there as well.

The wbinvd in arch/x86/kernel/relocate_kernel_64.S is only performed if
SME is active, so that one won't be executed on an Intel chip.

Thanks,
Tom

> 
>>
>> I'm wondering if there is some issue with MCE generation and wbinvd
>> and whatever, and doing it when the CPU is down and interrupts are
>> disabled causes some system issue..
>>
>> Does anybody have any other ideas?
>>
>> Linus

Re: [PATCH 3/3] arm64: Add work around for Arm Cortex-A55 Erratum 1024718

[Channa]

On 2018-01-17 02:28, Suzuki K Poulose wrote:

On 17/01/18 03:34, ckad...@codeaurora.org wrote:

On 2018-01-16 02:23, Suzuki K Poulose wrote:

Some variants of the Arm Cortex-55 cores (r0p0, r0p1, r1p0) suffer
from an erratum 1024718, which causes incorrect updates when DBM/AP
bits in a page table entry is modified without a break-before-make
sequence. The work around is to disable the hardware DBM feature
on the affected cores. The hardware Access Flag management features
is not affected.

The hardware DBM feature is a non-conflicting capability, i.e, the
kernel could handle cores using the feature and those without having
the features running at the same time. So this work around is 
detected
at early boot time, rather than delaying it until the CPUs are 
brought
up into the kernel with MMU turned on. This also avoids other 
complexities
with late CPUs turning online, with or without the hardware DBM 
features.


Cc: Catalin Marinas 
Cc: Mark Rutland 
Cc: Will Deacon 
Signed-off-by: Suzuki K Poulose 
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 14 ++
 arch/arm64/mm/proc.S   |  5 +
 3 files changed, 20 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.txt
b/Documentation/arm64/silicon-errata.txt
index b9d93e981a05..5203e71c113d 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -55,6 +55,7 @@ stable kernels.
 | ARM    | Cortex-A57  | #834220 |
ARM64_ERRATUM_834220    |
 | ARM    | Cortex-A72  | #853709 | N/A
 |
 | ARM    | Cortex-A73  | #858921 |
ARM64_ERRATUM_858921    |
+| ARM    | Cortex-A55  | #1024718    |
ARM64_ERRATUM_1024718   |
 | ARM    | MMU-500 | #841119,#826419 | N/A
 |
 |    | | |
 |
 | Cavium | ThunderX ITS    | #22375, #24313  |
CAVIUM_ERRATUM_22375    |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 664fadc2aa2e..19b8407a0325 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,20 @@ config ARM64_ERRATUM_843419

   If unsure, say Y.

+config ARM64_ERRATUM_1024718
+    bool "Cortex-A55: 1024718: Update of DBM/AP bits without break
before make might result in incorrect update"
+    default y
+    help
+  This option adds work around for Arm Cortex-A55 Erratum 
1024718.

+
+  Affected Cortex-A55 cores (r0p0, r0p1, r1p0) could cause 
incorrect
+  update of the hardware dirty bit when the DBM/AP bits are 
updated
+  without a break-before-make. The work around is to disable the 
usage
+  of hardware DBM locally on the affected cores. CPUs not 
affected by

+  erratum will continue to use the feature.
+
+  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 bool "Cavium erratum 22375, 24313"
 default y
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5a59eea49395..ba2c22180f4e 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -252,6 +252,11 @@ ENTRY(__cpu_setup)
 cbz    x9, 2f
 cmp    x9, #2
 b.lt    1f
+#ifdef CONFIG_ARM64_ERRATUM_1024718
+    /* Disable hardware DBM on Cortex-A55 r0p0, r0p1 & r1p0 */
+    cpu_midr_match MIDR_CORTEX_A55, MIDR_CPU_VAR_REV(0, 0),


What is there is a custom core with different MIDRs, can we specify 
multiple MIDR values?


At the moment no. May be we could pass a table of such values to the 
macro ?


Would it be good to clear the bit as part of 
arch/arm64/kernel/cpu_errata.c so we can specify multiple MIDR values 
if required.


The problem is, we already have some part of the kernel mappings with
PTE_DBM set
(PTE_WRITE = PTE_DBM with CONFIG_HW_AFDBM) and could potentially hit 
the errata,

before we disable it on the CPU. Also, if the CPU is brought up late
by userspace,
that adds more entities. I had another approach, where we delay 
enabling the
TCR_HD until all cores are up. But then it has other complexities with 
the CPU

feature framework.
e.g, we can't use the feature unless we turn the HADBS feature bit to
HIGHER_SAFE
so that we can turn it on if at least one CPU has it. But then, we 
don't know
what the future values of the feature could imply, leaving that choice 
unsafe.
Also, a late CPU will be prevented from booting if it doesn't have DBM 
unless

we hack the framework.


I was thinking if we can enable the DBM feature based on a cpu feature 
register.
Not sure if all future CPUs would have a bit for identifying whether DBM 
is supported

or not.



So an early check seemed the easier solution at the moment. I will take 
a look
at changing the framework a little bit and see where it takes us. 
Otherwise,

we could switch back to a table of affected MIDRs.


Agree, its better to change the implementation to take a 

Re: [PATCH 3/3] arm64: Add work around for Arm Cortex-A55 Erratum 1024718

[Channa]

On 2018-01-17 02:28, Suzuki K Poulose wrote:

On 17/01/18 03:34, ckad...@codeaurora.org wrote:

On 2018-01-16 02:23, Suzuki K Poulose wrote:

Some variants of the Arm Cortex-55 cores (r0p0, r0p1, r1p0) suffer
from an erratum 1024718, which causes incorrect updates when DBM/AP
bits in a page table entry is modified without a break-before-make
sequence. The work around is to disable the hardware DBM feature
on the affected cores. The hardware Access Flag management features
is not affected.

The hardware DBM feature is a non-conflicting capability, i.e, the
kernel could handle cores using the feature and those without having
the features running at the same time. So this work around is 
detected
at early boot time, rather than delaying it until the CPUs are 
brought
up into the kernel with MMU turned on. This also avoids other 
complexities
with late CPUs turning online, with or without the hardware DBM 
features.


Cc: Catalin Marinas 
Cc: Mark Rutland 
Cc: Will Deacon 
Signed-off-by: Suzuki K Poulose 
---
 Documentation/arm64/silicon-errata.txt |  1 +
 arch/arm64/Kconfig | 14 ++
 arch/arm64/mm/proc.S   |  5 +
 3 files changed, 20 insertions(+)

diff --git a/Documentation/arm64/silicon-errata.txt
b/Documentation/arm64/silicon-errata.txt
index b9d93e981a05..5203e71c113d 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -55,6 +55,7 @@ stable kernels.
 | ARM    | Cortex-A57  | #834220 |
ARM64_ERRATUM_834220    |
 | ARM    | Cortex-A72  | #853709 | N/A
 |
 | ARM    | Cortex-A73  | #858921 |
ARM64_ERRATUM_858921    |
+| ARM    | Cortex-A55  | #1024718    |
ARM64_ERRATUM_1024718   |
 | ARM    | MMU-500 | #841119,#826419 | N/A
 |
 |    | | |
 |
 | Cavium | ThunderX ITS    | #22375, #24313  |
CAVIUM_ERRATUM_22375    |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 664fadc2aa2e..19b8407a0325 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -461,6 +461,20 @@ config ARM64_ERRATUM_843419

   If unsure, say Y.

+config ARM64_ERRATUM_1024718
+    bool "Cortex-A55: 1024718: Update of DBM/AP bits without break
before make might result in incorrect update"
+    default y
+    help
+  This option adds work around for Arm Cortex-A55 Erratum 
1024718.

+
+  Affected Cortex-A55 cores (r0p0, r0p1, r1p0) could cause 
incorrect
+  update of the hardware dirty bit when the DBM/AP bits are 
updated
+  without a break-before-make. The work around is to disable the 
usage
+  of hardware DBM locally on the affected cores. CPUs not 
affected by

+  erratum will continue to use the feature.
+
+  If unsure, say Y.
+
 config CAVIUM_ERRATUM_22375
 bool "Cavium erratum 22375, 24313"
 default y
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5a59eea49395..ba2c22180f4e 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -252,6 +252,11 @@ ENTRY(__cpu_setup)
 cbz    x9, 2f
 cmp    x9, #2
 b.lt    1f
+#ifdef CONFIG_ARM64_ERRATUM_1024718
+    /* Disable hardware DBM on Cortex-A55 r0p0, r0p1 & r1p0 */
+    cpu_midr_match MIDR_CORTEX_A55, MIDR_CPU_VAR_REV(0, 0),


What is there is a custom core with different MIDRs, can we specify 
multiple MIDR values?


At the moment no. May be we could pass a table of such values to the 
macro ?


Would it be good to clear the bit as part of 
arch/arm64/kernel/cpu_errata.c so we can specify multiple MIDR values 
if required.


The problem is, we already have some part of the kernel mappings with
PTE_DBM set
(PTE_WRITE = PTE_DBM with CONFIG_HW_AFDBM) and could potentially hit 
the errata,

before we disable it on the CPU. Also, if the CPU is brought up late
by userspace,
that adds more entities. I had another approach, where we delay 
enabling the
TCR_HD until all cores are up. But then it has other complexities with 
the CPU

feature framework.
e.g, we can't use the feature unless we turn the HADBS feature bit to
HIGHER_SAFE
so that we can turn it on if at least one CPU has it. But then, we 
don't know
what the future values of the feature could imply, leaving that choice 
unsafe.
Also, a late CPU will be prevented from booting if it doesn't have DBM 
unless

we hack the framework.


I was thinking if we can enable the DBM feature based on a cpu feature 
register.
Not sure if all future CPUs would have a bit for identifying whether DBM 
is supported

or not.



So an early check seemed the easier solution at the moment. I will take 
a look
at changing the framework a little bit and see where it takes us. 
Otherwise,

we could switch back to a table of affected MIDRs.


Agree, its better to change the implementation to take a table of MIDRs.



Suzuki


--
--
The Qualcomm Innovation Center, Inc. is a member of the Code