Re: namei_vfat.c array subscript is above array bounds

[OGAWA Hirofumi]

Meelis Roos  writes:

>> > I do not know if this is old or new, just noticed it scrolling by while 
>> > compiling current 4.12+git on 32-bit x86.
>> 
>> Which version of compiler?
>
> gcc version 6.4.0 20170704 (Debian 6.4.0-1)
>
> After "touch fs/fat/namei_vfat.c" it appears consitently for me. Debian 
> unstable, up to date.

Tried with same gcc version, I can't still reproduce it. Possible
difference is, I'm using x86_64 gcc with ARCH=i386 (-m32).

$ md5sum fs/fat/namei_vfat.c arch/x86/include/asm/string*
d8994e21f6ebc27f89b30a2532b0bc1d  fs/fat/namei_vfat.c
e174dd82e3956479e76f2b78ba397287  arch/x86/include/asm/string.h
662b134b5adb0919ea23e95752a18290  arch/x86/include/asm/string_32.h
b91bcf192dcde29933275f0901e5dac5  arch/x86/include/asm/string_64.h

Source is same with above?
-- 
OGAWA Hirofumi 

Re: namei_vfat.c array subscript is above array bounds

[OGAWA Hirofumi]

Meelis Roos  writes:

>> > I do not know if this is old or new, just noticed it scrolling by while 
>> > compiling current 4.12+git on 32-bit x86.
>> 
>> Which version of compiler?
>
> gcc version 6.4.0 20170704 (Debian 6.4.0-1)
>
> After "touch fs/fat/namei_vfat.c" it appears consitently for me. Debian 
> unstable, up to date.

Tried with same gcc version, I can't still reproduce it. Possible
difference is, I'm using x86_64 gcc with ARCH=i386 (-m32).

$ md5sum fs/fat/namei_vfat.c arch/x86/include/asm/string*
d8994e21f6ebc27f89b30a2532b0bc1d  fs/fat/namei_vfat.c
e174dd82e3956479e76f2b78ba397287  arch/x86/include/asm/string.h
662b134b5adb0919ea23e95752a18290  arch/x86/include/asm/string_32.h
b91bcf192dcde29933275f0901e5dac5  arch/x86/include/asm/string_64.h

Source is same with above?
-- 
OGAWA Hirofumi 

Re: [PATCH v16 2/7] power: add power sequence library

[Peter Chen]

On Fri, Jul 07, 2017 at 03:03:06PM +0200, Rafael J. Wysocki wrote:
> On Friday, July 07, 2017 04:01:07 PM Peter Chen wrote:
> > On Fri, Jul 07, 2017 at 03:13:48AM +0200, Rafael J. Wysocki wrote:
> > > > 
> > > > - Can I write new code for it or I need to depend on something?
> > > 
> > > There is nothing this code needs to depend on AFAICS, but there are 
> > > existing
> > > solutions in this problem space (ACPI power management, genpd), so it 
> > > needs to
> > > be careful enough about possible overlaps etc.
> > > 
> > > > I find there is already "power state" concept at documentation.
> > > > Documentation/ABI/testing/sysfs-devices-power_state
> > > 
> > > This is ACPI-specific and only in sysfs directories representing ACPI 
> > > device
> > > objects (which aren't physical devices).
> > > 
> > > Anyway, since ACPI covers the problem space you are working in already,
> > > your code has to be mutually exclusive with it.
> > > 
> > > > - If I can write the new code for it, except the problems I want
> > > > to fix, are there any other use cases I need to consider?
> > > 
> > > I would start simple and focus on the particular problem at hand, that is
> > > devices with two power states ("on" and "off") where the "on" state
> > > depends on a number of clocks and/or GPIOs.  Still, I'd also avoid making
> > > design choices that might prevent it from being extended in the future
> > > if need be.
> > > 
> > > One major problem I can see is how to "attach" the power states framework
> > > to a particular device (once we have discovered that it should be used 
> > > with
> > > that device).
> > > 
> > > For bus types that don't do power management of their own you could follow
> > > ACPI (and genpd) and provide a PM domain for this purpose, but bus types
> > > doing their own PM (like USB) will probably need to be treated 
> > > differently.
> > > In those cases the bus type code will have to know that it should call 
> > > some
> > > helpers to switch power states of devices.
> > > 
> > 
> > After thinking more, using a power state framework is seems too heavy
> > for this use case. This use case is just do some clock and gpio
> > operations before device is created, and do some put operations
> > after device is deleted. We just need some helpers in one structure
> > (called "power sequence" or "power state") for this purpose.
> > 
> > For the use case, the clock and gpio operation can be done after device
> > is created, the power domain is more suitable.
> 
> There is a problem with PM domains that they only provide hooks for runtime PM
> and system suspend/resume (including hibernation) and not for generic
> "power up" and "power down" operations that may need to be carried out at
> probe time before the runtime PM framework can be used (and analogously
> at remove time).
> 
> I would consider starting with the patch below or similar.
> 
> Then you can define something like POWER_STATE_SEQUENCE type for your
> case and basically use almost what you have already with it, except that
> struct pwrsec_generic will now become struct power_state_sequence and
> struct power_state_info will be embedded in it instead of struct pwrsec.
> 
> The major comceptual difference is that ->power_up and ->power_down are
> now available at the level of the device that needs the power sequence and
> pm_device_power_up/down() can be used wherever necessary (in the code,
> in a bus type, in a controller driver or even in the driver for this 
> particular
> device).

Rafeal, thanks for your patch.

The biggest problem for my use case is the device is still not created.
How can I call pm_device_power_up(dev)?

Peter
> 
> Most likely you will need a PM domain in addition to that, but mostly to avoid
> code duplication.
> 
> And in the future other types of power state definitions may be hooked up
> to that, including ACPI etc.
> 
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/base/power/common.c |   35 +++
>  include/linux/pm.h  |   33 +
>  2 files changed, 68 insertions(+)
> 
> Index: linux-pm/include/linux/pm.h
> ===
> --- linux-pm.orig/include/linux/pm.h
> +++ linux-pm/include/linux/pm.h
> @@ -550,6 +550,30 @@ struct pm_subsys_data {
>  #endif
>  };
>  
> +enum power_state_type {
> + POWER_STATE_GENERIC = 0,
> +};
> +
> +/**
> + * struct power_state_info - information related to device power states.
> + *
> + * @type: Power states definition type.
> + * @power_up: Device power up method.
> + * @power_down: Device power down method.
> + *
> + * @power_up is expected to put the device into a power state in which it can
> + * be operated by software (it doesn't have to be the full power state in
> + * principle as long as the device will respond to all software accesses in
> + * this state) and @power_down is expected to put the device into the lowest

Re: [PATCH v16 2/7] power: add power sequence library

[Peter Chen]

On Fri, Jul 07, 2017 at 03:03:06PM +0200, Rafael J. Wysocki wrote:
> On Friday, July 07, 2017 04:01:07 PM Peter Chen wrote:
> > On Fri, Jul 07, 2017 at 03:13:48AM +0200, Rafael J. Wysocki wrote:
> > > > 
> > > > - Can I write new code for it or I need to depend on something?
> > > 
> > > There is nothing this code needs to depend on AFAICS, but there are 
> > > existing
> > > solutions in this problem space (ACPI power management, genpd), so it 
> > > needs to
> > > be careful enough about possible overlaps etc.
> > > 
> > > > I find there is already "power state" concept at documentation.
> > > > Documentation/ABI/testing/sysfs-devices-power_state
> > > 
> > > This is ACPI-specific and only in sysfs directories representing ACPI 
> > > device
> > > objects (which aren't physical devices).
> > > 
> > > Anyway, since ACPI covers the problem space you are working in already,
> > > your code has to be mutually exclusive with it.
> > > 
> > > > - If I can write the new code for it, except the problems I want
> > > > to fix, are there any other use cases I need to consider?
> > > 
> > > I would start simple and focus on the particular problem at hand, that is
> > > devices with two power states ("on" and "off") where the "on" state
> > > depends on a number of clocks and/or GPIOs.  Still, I'd also avoid making
> > > design choices that might prevent it from being extended in the future
> > > if need be.
> > > 
> > > One major problem I can see is how to "attach" the power states framework
> > > to a particular device (once we have discovered that it should be used 
> > > with
> > > that device).
> > > 
> > > For bus types that don't do power management of their own you could follow
> > > ACPI (and genpd) and provide a PM domain for this purpose, but bus types
> > > doing their own PM (like USB) will probably need to be treated 
> > > differently.
> > > In those cases the bus type code will have to know that it should call 
> > > some
> > > helpers to switch power states of devices.
> > > 
> > 
> > After thinking more, using a power state framework is seems too heavy
> > for this use case. This use case is just do some clock and gpio
> > operations before device is created, and do some put operations
> > after device is deleted. We just need some helpers in one structure
> > (called "power sequence" or "power state") for this purpose.
> > 
> > For the use case, the clock and gpio operation can be done after device
> > is created, the power domain is more suitable.
> 
> There is a problem with PM domains that they only provide hooks for runtime PM
> and system suspend/resume (including hibernation) and not for generic
> "power up" and "power down" operations that may need to be carried out at
> probe time before the runtime PM framework can be used (and analogously
> at remove time).
> 
> I would consider starting with the patch below or similar.
> 
> Then you can define something like POWER_STATE_SEQUENCE type for your
> case and basically use almost what you have already with it, except that
> struct pwrsec_generic will now become struct power_state_sequence and
> struct power_state_info will be embedded in it instead of struct pwrsec.
> 
> The major comceptual difference is that ->power_up and ->power_down are
> now available at the level of the device that needs the power sequence and
> pm_device_power_up/down() can be used wherever necessary (in the code,
> in a bus type, in a controller driver or even in the driver for this 
> particular
> device).

Rafeal, thanks for your patch.

The biggest problem for my use case is the device is still not created.
How can I call pm_device_power_up(dev)?

Peter
> 
> Most likely you will need a PM domain in addition to that, but mostly to avoid
> code duplication.
> 
> And in the future other types of power state definitions may be hooked up
> to that, including ACPI etc.
> 
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/base/power/common.c |   35 +++
>  include/linux/pm.h  |   33 +
>  2 files changed, 68 insertions(+)
> 
> Index: linux-pm/include/linux/pm.h
> ===
> --- linux-pm.orig/include/linux/pm.h
> +++ linux-pm/include/linux/pm.h
> @@ -550,6 +550,30 @@ struct pm_subsys_data {
>  #endif
>  };
>  
> +enum power_state_type {
> + POWER_STATE_GENERIC = 0,
> +};
> +
> +/**
> + * struct power_state_info - information related to device power states.
> + *
> + * @type: Power states definition type.
> + * @power_up: Device power up method.
> + * @power_down: Device power down method.
> + *
> + * @power_up is expected to put the device into a power state in which it can
> + * be operated by software (it doesn't have to be the full power state in
> + * principle as long as the device will respond to all software accesses in
> + * this state) and @power_down is expected to put the device into the lowest
> + * power state the device 

Re: [PATCH] printk: Move printk_delay to separate file

[Joe Perches]

On Sat, 2017-07-08 at 14:24 +0900, Sergey Senozhatsky wrote:
> On (07/07/17 11:08), Joe Perches wrote:
> > printk.c is a huge file with too many local functions for a
> > human to read and easily parse.
> > 
> > Start to separate out bits into smaller files.
> > 
> > Miscellanea:
> > 
> > o Rename suppress_message_printing to printk_suppress_message
> > o Add function definitions to printk.h
> 
> I don't mind, in general, but I'm a bit hesitant. we want to have
> automatic printk throttling (printk delay basically) and we need
> some of those printk-internal *_seq numbers to see how far consoles
> are behind the logbuf. so either we need to 'un-static' those *_seq
> and extern them in delay.c or simply keep printk-delay machinery in
> printk.c and add the new function.
> 
> // p.s. I'll take a look at the patch a bit later. I'm on a sick leave now.

Hey Sergey.

Basically, this is a simple trial patch.

printk is getting nothing but more complex.

I believe printk is in real need of logical separation
into multiple parts to isolate the various logic bits.

o console
o kmsg/devkmsg
o logbuf
o syslog

etc...

Re: [PATCH] printk: Move printk_delay to separate file

[Joe Perches]

On Sat, 2017-07-08 at 14:24 +0900, Sergey Senozhatsky wrote:
> On (07/07/17 11:08), Joe Perches wrote:
> > printk.c is a huge file with too many local functions for a
> > human to read and easily parse.
> > 
> > Start to separate out bits into smaller files.
> > 
> > Miscellanea:
> > 
> > o Rename suppress_message_printing to printk_suppress_message
> > o Add function definitions to printk.h
> 
> I don't mind, in general, but I'm a bit hesitant. we want to have
> automatic printk throttling (printk delay basically) and we need
> some of those printk-internal *_seq numbers to see how far consoles
> are behind the logbuf. so either we need to 'un-static' those *_seq
> and extern them in delay.c or simply keep printk-delay machinery in
> printk.c and add the new function.
> 
> // p.s. I'll take a look at the patch a bit later. I'm on a sick leave now.

Hey Sergey.

Basically, this is a simple trial patch.

printk is getting nothing but more complex.

I believe printk is in real need of logical separation
into multiple parts to isolate the various logic bits.

o console
o kmsg/devkmsg
o logbuf
o syslog

etc...

[GIT PULL]: dmaengine updates for 4.13-rc1

[Vinod Koul]

Hi Linus,

Please pull dmaengine updates for v4.13-rc1 as detailed below.

The AVR32 removal in dma and sound has been coordinated with me pulling
tiwai/topic/avr32-removal into dmaengine tree, you should get those from
Takashi.

Also the mv_xor fixes ai/topic/avr32-removal is already merged in 4.12 as
fixes. The updates are in this request, so diffstat and patches for those
don't reflect correctly as this pull is still based on 4.12-rc1.

The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.infradead.org/users/vkoul/slave-dma.git tags/dmaengine-4.13-rc1

for you to fetch changes up to 3edc85023a1e1daf22d8e372c5c4f87dc4a04a71:

  Merge branch 'topic/rcar' into for-linus (2017-07-04 10:05:22 +0530)


dmaengine updates for 4.13-rc1

 - removal of AVR32 support in dw driver as AVR32 is gone
 - new driver for Broadcom stream buffer accelerator (SBA) RAID driver
 - add support for Faraday Technology FTDMAC020 in amba-pl08x driver
 - IOMMU support in pl330 driver
 - updates to bunch of drivers


Andy Shevchenko (2):
  ALSA: atmel: Remove AVR32 bits from the driver
  dmaengine: dw: Remove AVR32 bits from the driver

Anup Patel (4):
  lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
  async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
  dmaengine: Add Broadcom SBA RAID driver
  dt-bindings: Add DT bindings document for Broadcom SBA RAID driver

Arnd Bergmann (1):
  dmaengine: bcm-sba-raid: fix Kconfig dependencies

Arvind Yadav (4):
  dmaengine: DW DMAC: Handle return value of clk_prepare_enable
  dmaengine: imx-sdma: Handle return value of clk_prepare_enable
  dmaengine: imx-sdma: Fix compilation warning.
  dmaengine: fsl_raid: make of_device_ids const.

Colin Ian King (1):
  dmaengine: bcm-scm-raid: remove redundant null check on req

Fabio Estevam (3):
  dmaengine: mxs: Use %zu for printing a size_t variable
  dmaengine: Kconfig: Extend the dependency for MXS_DMA
  dmaengine: Kconfig: Simplify the help text for MXS_DMA

Geert Uytterhoeven (1):
  dt-bindings: rcar-dmac: Document missing error interrupt

Hanna Hawa (4):
  dmaengine: mv_xor_v2: enable XOR engine after its configuration
  dmaengine: mv_xor_v2: implement proper interrupt coalescing
  dmaengine: mv_xor_v2: remove unnecessary write to DESQ_STOP register
  dmaengine: mv_xor_v2: add support for suspend/resume

Jon Hunter (1):
  dmaengine: tegra-apb: Really fix runtime-pm usage

Linus Walleij (5):
  ARM/dmaengine: pl08x: pass reasonable memcpy settings
  dmaengine: pl08x: Add Faraday FTDMAC020 to compatible list
  dmaengine: pl08x: Make slave engine optional
  dmaengine: pl08x: Add support for Faraday Technology FTDMAC020
  dmaengine: pl08x: use GENMASK() to create bitmasks

Logan Gunthorpe (2):
  dmaengine: imx-dma: cleanup scatterlist layering violations
  dmaengine: ste_dma40: Cleanup scatterlist layering violations

Matthias Kaehlcke (1):
  dmaengine: pl330: Delete unused functions

Niklas Söderlund (3):
  dmaengine: rcar-dmac: store channel IRQ in struct rcar_dmac_chan
  dmaengine: rcar-dmac: implement device_synchronize()
  dmaengine: rcar-dmac: wait for ISR to finish before freeing resources

Prarit Bhargava (1):
  dmaengine: Replace WARN_TAINT_ONCE() with pr_warn_once()

Robin Murphy (1):
  dmaengine: pl330: Add IOMMU support to slave tranfers

Sinan Kaya (2):
  dmaengine: qcom_hidma: allow ACPI/DT parameters to be overridden
  dmaengine: qcom_hidma: correct API violation for submit

Stefan Roese (1):
  dmaengine: zynqmp_dma: Remove max len check in zynqmp_dma_prep_memcpy

Thomas Breitung (1):
  dmaengine: fsldma: set BWC, DAHTS and SAHTS values correctly

Thomas Petazzoni (6):
  dmaengine: mv_xor_v2: handle mv_xor_v2_prep_sw_desc() error properly
  dmaengine: mv_xor_v2: properly handle wrapping in the array of HW 
descriptors
  dmaengine: mv_xor_v2: do not use descriptors not acked by async_tx
  dmaengine: mv_xor_v2: fix tx_submit() implementation
  dmaengine: mv_xor_v2: remove interrupt coalescing
  dmaengine: mv_xor_v2: set DMA mask to 40 bits

Vinod Koul (7):
  Merge branch 'topic/bcm' into for-linus
  Merge branch 'topic/dw' into for-linus
  Merge branch 'topic/mv_xor' into for-linus
  Merge branch 'topic/mxs' into for-linus
  Merge branch 'topic/pl08x' into for-linus
  Merge branch 'topic/pl330' into for-linus
  Merge branch 'topic/rcar' into for-linus

Wolfram Sang (1):
  dmaengine: use proper name for the R-Car SoC

 .../devicetree/bindings/dma/arm-pl08x.txt  |9 +-
 .../devicetree/bindings/dma/brcm,iproc-sba.txt |   29 +
 

[GIT PULL]: dmaengine updates for 4.13-rc1

[Vinod Koul]

Hi Linus,

Please pull dmaengine updates for v4.13-rc1 as detailed below.

The AVR32 removal in dma and sound has been coordinated with me pulling
tiwai/topic/avr32-removal into dmaengine tree, you should get those from
Takashi.

Also the mv_xor fixes ai/topic/avr32-removal is already merged in 4.12 as
fixes. The updates are in this request, so diffstat and patches for those
don't reflect correctly as this pull is still based on 4.12-rc1.

The following changes since commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6:

  Linux 4.12-rc1 (2017-05-13 13:19:49 -0700)

are available in the git repository at:

  git://git.infradead.org/users/vkoul/slave-dma.git tags/dmaengine-4.13-rc1

for you to fetch changes up to 3edc85023a1e1daf22d8e372c5c4f87dc4a04a71:

  Merge branch 'topic/rcar' into for-linus (2017-07-04 10:05:22 +0530)


dmaengine updates for 4.13-rc1

 - removal of AVR32 support in dw driver as AVR32 is gone
 - new driver for Broadcom stream buffer accelerator (SBA) RAID driver
 - add support for Faraday Technology FTDMAC020 in amba-pl08x driver
 - IOMMU support in pl330 driver
 - updates to bunch of drivers


Andy Shevchenko (2):
  ALSA: atmel: Remove AVR32 bits from the driver
  dmaengine: dw: Remove AVR32 bits from the driver

Anup Patel (4):
  lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
  async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
  dmaengine: Add Broadcom SBA RAID driver
  dt-bindings: Add DT bindings document for Broadcom SBA RAID driver

Arnd Bergmann (1):
  dmaengine: bcm-sba-raid: fix Kconfig dependencies

Arvind Yadav (4):
  dmaengine: DW DMAC: Handle return value of clk_prepare_enable
  dmaengine: imx-sdma: Handle return value of clk_prepare_enable
  dmaengine: imx-sdma: Fix compilation warning.
  dmaengine: fsl_raid: make of_device_ids const.

Colin Ian King (1):
  dmaengine: bcm-scm-raid: remove redundant null check on req

Fabio Estevam (3):
  dmaengine: mxs: Use %zu for printing a size_t variable
  dmaengine: Kconfig: Extend the dependency for MXS_DMA
  dmaengine: Kconfig: Simplify the help text for MXS_DMA

Geert Uytterhoeven (1):
  dt-bindings: rcar-dmac: Document missing error interrupt

Hanna Hawa (4):
  dmaengine: mv_xor_v2: enable XOR engine after its configuration
  dmaengine: mv_xor_v2: implement proper interrupt coalescing
  dmaengine: mv_xor_v2: remove unnecessary write to DESQ_STOP register
  dmaengine: mv_xor_v2: add support for suspend/resume

Jon Hunter (1):
  dmaengine: tegra-apb: Really fix runtime-pm usage

Linus Walleij (5):
  ARM/dmaengine: pl08x: pass reasonable memcpy settings
  dmaengine: pl08x: Add Faraday FTDMAC020 to compatible list
  dmaengine: pl08x: Make slave engine optional
  dmaengine: pl08x: Add support for Faraday Technology FTDMAC020
  dmaengine: pl08x: use GENMASK() to create bitmasks

Logan Gunthorpe (2):
  dmaengine: imx-dma: cleanup scatterlist layering violations
  dmaengine: ste_dma40: Cleanup scatterlist layering violations

Matthias Kaehlcke (1):
  dmaengine: pl330: Delete unused functions

Niklas Söderlund (3):
  dmaengine: rcar-dmac: store channel IRQ in struct rcar_dmac_chan
  dmaengine: rcar-dmac: implement device_synchronize()
  dmaengine: rcar-dmac: wait for ISR to finish before freeing resources

Prarit Bhargava (1):
  dmaengine: Replace WARN_TAINT_ONCE() with pr_warn_once()

Robin Murphy (1):
  dmaengine: pl330: Add IOMMU support to slave tranfers

Sinan Kaya (2):
  dmaengine: qcom_hidma: allow ACPI/DT parameters to be overridden
  dmaengine: qcom_hidma: correct API violation for submit

Stefan Roese (1):
  dmaengine: zynqmp_dma: Remove max len check in zynqmp_dma_prep_memcpy

Thomas Breitung (1):
  dmaengine: fsldma: set BWC, DAHTS and SAHTS values correctly

Thomas Petazzoni (6):
  dmaengine: mv_xor_v2: handle mv_xor_v2_prep_sw_desc() error properly
  dmaengine: mv_xor_v2: properly handle wrapping in the array of HW 
descriptors
  dmaengine: mv_xor_v2: do not use descriptors not acked by async_tx
  dmaengine: mv_xor_v2: fix tx_submit() implementation
  dmaengine: mv_xor_v2: remove interrupt coalescing
  dmaengine: mv_xor_v2: set DMA mask to 40 bits

Vinod Koul (7):
  Merge branch 'topic/bcm' into for-linus
  Merge branch 'topic/dw' into for-linus
  Merge branch 'topic/mv_xor' into for-linus
  Merge branch 'topic/mxs' into for-linus
  Merge branch 'topic/pl08x' into for-linus
  Merge branch 'topic/pl330' into for-linus
  Merge branch 'topic/rcar' into for-linus

Wolfram Sang (1):
  dmaengine: use proper name for the R-Car SoC

 .../devicetree/bindings/dma/arm-pl08x.txt  |9 +-
 .../devicetree/bindings/dma/brcm,iproc-sba.txt |   29 +
 

Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure for SCMI

[Jassi Brar]

Hi Roy, Matt, Nishant, Harb Abdulhamid, Loc,

I have a gut feeling you guys were part of the SCMI spec committee. If
so, could you please chime in?


On Fri, Jul 7, 2017 at 11:09 PM, Sudeep Holla  wrote:
>
>
> On 07/07/17 17:52, Jassi Brar wrote:
>> Hi Arnd, Hi Rob, Hi Mark,
>>
>> [CC'ing only those who I have the email id of]
>>
>>> +/**
>>> + * scmi_do_xfer() - Do one transfer
>>> + *
>>> + * @info: Pointer to SCMI entity information
>>> + * @xfer: Transfer to initiate and wait for response
>>> + *
>>> + * Return: -ETIMEDOUT in case of no response, if transmit error,
>>> + *   return corresponding error, else if all goes well,
>>> + *   return 0.
>>> + */
>>> +int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
>>> +{
>>> +int ret;
>>> +int timeout;
>>> +struct scmi_info *info = handle_to_scmi_info(handle);
>>> +struct device *dev = info->dev;
>>> +
>>> +ret = mbox_send_message(info->tx_chan, xfer);
>>> +
>>>
>> The api is
>>
>> int mbox_send_message(struct mbox_chan *chan, void *mssg)
>>
>> where each controller driver defines its own format in which it accepts
>> the 'mssg' to be transmitted.
>>
>
> Yes they can continue that, but SCMI just doesn't depend on that.
>
>> For example :-
>> ti_msgmgr_send_data(struct mbox_chan *, struct ti_msgmgr_message *)
>> rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *)
>> and so on...  you get the idea.
>>
>
> Yes I am aware of that.
>
>> Some controller driver may ignore the 'mssg' because only an interrupt line
>> is shared with the remote and not some register/fifo.
>> For example,
>>   sti_mbox_send_data(struct mbox_chan *, void *ignored)
>>
>
> Exactly, now with SCMI, every controller *can do* that, as we just care
> about the signaling which in other terms I have so far referred as
> "doorbell".
>
No, the controllers can not ... unless you clone and adapt the 9
drivers+bindings to conform to the expectations of SCMI (like you
attempted with MHU recently). Also, then mandate every future
controller driver must emulate "doorbell" channels.

As the mailbox maintainer, I am open to suggestions that would allow
every controller to support SCMI.
But compared to the options of scmi-as-a-library and
scmi-as-child-node-of-platform-parent, this does not even qualify as
an option.

Why? Because SCMI is but one protocol that provides 4 features ATM,
and certainly can not provide for every whim and quirk of future
platforms.  Among the sane requirements are watchdog,
suspend/resume/hibernation and thermal _control_ (not just sensor
readings) and among the weird are video, network and storage over
mailbox api. And even a filesystem backed by read/write over mailbox!!
 And these are only that I have worked on first hand.

  The point is : you can not assume SCMI to be the only protocol
running over a controller _and_ you can not dictate other protocols to
not touch certain bits of the signal register/fifo.

In simplest terms, controller driver can not cater to only a
particular client. That's the reason we have the controller driver
define the message format and clients conform to it.

Thanks.

Re: [PATCH 3/9] firmware: arm_scmi: add basic driver infrastructure for SCMI

[Jassi Brar]

Hi Roy, Matt, Nishant, Harb Abdulhamid, Loc,

I have a gut feeling you guys were part of the SCMI spec committee. If
so, could you please chime in?


On Fri, Jul 7, 2017 at 11:09 PM, Sudeep Holla  wrote:
>
>
> On 07/07/17 17:52, Jassi Brar wrote:
>> Hi Arnd, Hi Rob, Hi Mark,
>>
>> [CC'ing only those who I have the email id of]
>>
>>> +/**
>>> + * scmi_do_xfer() - Do one transfer
>>> + *
>>> + * @info: Pointer to SCMI entity information
>>> + * @xfer: Transfer to initiate and wait for response
>>> + *
>>> + * Return: -ETIMEDOUT in case of no response, if transmit error,
>>> + *   return corresponding error, else if all goes well,
>>> + *   return 0.
>>> + */
>>> +int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer)
>>> +{
>>> +int ret;
>>> +int timeout;
>>> +struct scmi_info *info = handle_to_scmi_info(handle);
>>> +struct device *dev = info->dev;
>>> +
>>> +ret = mbox_send_message(info->tx_chan, xfer);
>>> +
>>>
>> The api is
>>
>> int mbox_send_message(struct mbox_chan *chan, void *mssg)
>>
>> where each controller driver defines its own format in which it accepts
>> the 'mssg' to be transmitted.
>>
>
> Yes they can continue that, but SCMI just doesn't depend on that.
>
>> For example :-
>> ti_msgmgr_send_data(struct mbox_chan *, struct ti_msgmgr_message *)
>> rockchip_mbox_send_data(struct mbox_chan *, struct rockchip_mbox_msg *)
>> and so on...  you get the idea.
>>
>
> Yes I am aware of that.
>
>> Some controller driver may ignore the 'mssg' because only an interrupt line
>> is shared with the remote and not some register/fifo.
>> For example,
>>   sti_mbox_send_data(struct mbox_chan *, void *ignored)
>>
>
> Exactly, now with SCMI, every controller *can do* that, as we just care
> about the signaling which in other terms I have so far referred as
> "doorbell".
>
No, the controllers can not ... unless you clone and adapt the 9
drivers+bindings to conform to the expectations of SCMI (like you
attempted with MHU recently). Also, then mandate every future
controller driver must emulate "doorbell" channels.

As the mailbox maintainer, I am open to suggestions that would allow
every controller to support SCMI.
But compared to the options of scmi-as-a-library and
scmi-as-child-node-of-platform-parent, this does not even qualify as
an option.

Why? Because SCMI is but one protocol that provides 4 features ATM,
and certainly can not provide for every whim and quirk of future
platforms.  Among the sane requirements are watchdog,
suspend/resume/hibernation and thermal _control_ (not just sensor
readings) and among the weird are video, network and storage over
mailbox api. And even a filesystem backed by read/write over mailbox!!
 And these are only that I have worked on first hand.

  The point is : you can not assume SCMI to be the only protocol
running over a controller _and_ you can not dictate other protocols to
not touch certain bits of the signal register/fifo.

In simplest terms, controller driver can not cater to only a
particular client. That's the reason we have the controller driver
define the message format and clients conform to it.

Thanks.

Re: [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: set initial mode for vaudio

[Tony Lindgren]

* Sebastian Reichel  [170707 13:08]:
> Set default mode for vaudio, which may be left in standby mode
> if the system is booted via kexec from Android.

Acked-by: Tony Lindgren 

Re: [PATCH 2/2] ARM: dts: motorola-cpcap-mapphone: set initial mode for vaudio

[Tony Lindgren]

* Sebastian Reichel  [170707 13:08]:
> Set default mode for vaudio, which may be left in standby mode
> if the system is booted via kexec from Android.

Acked-by: Tony Lindgren 

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Tony Lindgren]

* Sebastian Reichel  [170707 13:08]:
> While working on the audio-codec I noticed, that the
> low power mode of the regulators are not properly
> supported. This fixes the issue for vaudio.

Yeah good catch:

Acked-by: Tony Lindgren 

Re: [PATCH 1/2] regulator: cpcap: Fix standby mode

[Tony Lindgren]

* Sebastian Reichel  [170707 13:08]:
> While working on the audio-codec I noticed, that the
> low power mode of the regulators are not properly
> supported. This fixes the issue for vaudio.

Yeah good catch:

Acked-by: Tony Lindgren 

Re: [PATCH 0/3] Motorola Droid 4 Audio Support

[Tony Lindgren]

* Sebastian Reichel  [170707 09:43]:
> Hi,
> 
> I got working sound on Droid 4 with mainline \o/. The codec is
> currently missing support for detecting if something has been
> plugged into the 3.5mm connector, since that seems to require
> some closed source firmware and needs further investigation. I
> think this can be added later.

Hey that's great! I'll give it a try this weekend.. Does that mean
that 3G voice calls work too now or is something more needed there?

For the CPCAP PMIC macro interrupts I think it's best to set up
a separate driver as it seems separate from the core CPCAP
functionality. So I think we can just move the unused "cpcap-m2"
IRQ banks out of motorola-cpcap.c and put them into a separate child
driver that loads it's firmware on init and provides interrupts for
the 3.5mm connector.

Regards,

Tony

Re: [git pull] vfs.git pile 13 - killing __copy_in_user()

[Linus Torvalds]

On Fri, Jul 7, 2017 at 5:29 PM, Al Viro  wrote:
> There used to be 6 places in the entire tree calling __copy_in_user(),
> all of them bogus.  Four got killed off in work.drm branch, this takes care of
> the remaining ones and kills the definition of that sucker.

This branch is garbage.

You seem to have merged in some *old* drm branch into it, so it has
all those drm commits that I merged earlier, except they are not the
*same* commits.

So I'm not pulling this. Ugly duplicate commits and just unnecessary
conflicts showing up again.

  Linus

Re: [PATCH 0/3] Motorola Droid 4 Audio Support

[Tony Lindgren]

* Sebastian Reichel  [170707 09:43]:
> Hi,
> 
> I got working sound on Droid 4 with mainline \o/. The codec is
> currently missing support for detecting if something has been
> plugged into the 3.5mm connector, since that seems to require
> some closed source firmware and needs further investigation. I
> think this can be added later.

Hey that's great! I'll give it a try this weekend.. Does that mean
that 3G voice calls work too now or is something more needed there?

For the CPCAP PMIC macro interrupts I think it's best to set up
a separate driver as it seems separate from the core CPCAP
functionality. So I think we can just move the unused "cpcap-m2"
IRQ banks out of motorola-cpcap.c and put them into a separate child
driver that loads it's firmware on init and provides interrupts for
the 3.5mm connector.

Regards,

Tony

Re: [git pull] vfs.git pile 13 - killing __copy_in_user()

[Linus Torvalds]

On Fri, Jul 7, 2017 at 5:29 PM, Al Viro  wrote:
> There used to be 6 places in the entire tree calling __copy_in_user(),
> all of them bogus.  Four got killed off in work.drm branch, this takes care of
> the remaining ones and kills the definition of that sucker.

This branch is garbage.

You seem to have merged in some *old* drm branch into it, so it has
all those drm commits that I merged earlier, except they are not the
*same* commits.

So I'm not pulling this. Ugly duplicate commits and just unnecessary
conflicts showing up again.

  Linus

Re: [PATCH] printk: Move printk_delay to separate file

[Sergey Senozhatsky]

On (07/07/17 11:08), Joe Perches wrote:
> printk.c is a huge file with too many local functions for a
> human to read and easily parse.
> 
> Start to separate out bits into smaller files.
> 
> Miscellanea:
> 
> o Rename suppress_message_printing to printk_suppress_message
> o Add function definitions to printk.h

I don't mind, in general, but I'm a bit hesitant. we want to have
automatic printk throttling (printk delay basically) and we need
some of those printk-internal *_seq numbers to see how far consoles
are behind the logbuf. so either we need to 'un-static' those *_seq
and extern them in delay.c or simply keep printk-delay machinery in
printk.c and add the new function.

// p.s. I'll take a look at the patch a bit later. I'm on a sick leave now.

-ss

Re: [PATCH] printk: Move printk_delay to separate file

[Sergey Senozhatsky]

On (07/07/17 11:08), Joe Perches wrote:
> printk.c is a huge file with too many local functions for a
> human to read and easily parse.
> 
> Start to separate out bits into smaller files.
> 
> Miscellanea:
> 
> o Rename suppress_message_printing to printk_suppress_message
> o Add function definitions to printk.h

I don't mind, in general, but I'm a bit hesitant. we want to have
automatic printk throttling (printk delay basically) and we need
some of those printk-internal *_seq numbers to see how far consoles
are behind the logbuf. so either we need to 'un-static' those *_seq
and extern them in delay.c or simply keep printk-delay machinery in
printk.c and add the new function.

// p.s. I'll take a look at the patch a bit later. I'm on a sick leave now.

-ss

Re: [PATCH] printk: Modify operators of printed_len

[Sergey Senozhatsky]

On (07/08/17 10:51), Pierre Kuo wrote:
> In 8b1742c9c207, we remove printk-recursion detection code in
> vprintk_emit(), where it is the first place that printed_len calculated.
> After removing above detection, it seems we can directly assign the
> result of log_output to printed_len.
> 
> Signed-off-by: Pierre Kuo 

Reviewed-by: Sergey Senozhatsky 

-ss

> ---
>  kernel/printk/printk.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fc47863..a2a8cac 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>   size_t text_len = 0;
>   enum log_flags lflags = 0;
>   unsigned long flags;
> - int printed_len = 0;
> + int printed_len;
>   bool in_sched = false;
>  
>   if (level == LOGLEVEL_SCHED) {
> @@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>   if (dict)
>   lflags |= LOG_PREFIX|LOG_NEWLINE;
>  
> - printed_len += log_output(facility, level, lflags, dict, dictlen, text, 
> text_len);
> + printed_len = log_output(facility, level, lflags, dict, dictlen, text, 
> text_len);
>  
>   logbuf_unlock_irqrestore(flags);
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH] printk: Modify operators of printed_len

[Sergey Senozhatsky]

On (07/08/17 10:51), Pierre Kuo wrote:
> In 8b1742c9c207, we remove printk-recursion detection code in
> vprintk_emit(), where it is the first place that printed_len calculated.
> After removing above detection, it seems we can directly assign the
> result of log_output to printed_len.
> 
> Signed-off-by: Pierre Kuo 

Reviewed-by: Sergey Senozhatsky 

-ss

> ---
>  kernel/printk/printk.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index fc47863..a2a8cac 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>   size_t text_len = 0;
>   enum log_flags lflags = 0;
>   unsigned long flags;
> - int printed_len = 0;
> + int printed_len;
>   bool in_sched = false;
>  
>   if (level == LOGLEVEL_SCHED) {
> @@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
>   if (dict)
>   lflags |= LOG_PREFIX|LOG_NEWLINE;
>  
> - printed_len += log_output(facility, level, lflags, dict, dictlen, text, 
> text_len);
> + printed_len = log_output(facility, level, lflags, dict, dictlen, text, 
> text_len);
>  
>   logbuf_unlock_irqrestore(flags);
>  
> -- 
> 1.7.9.5
> 

Re: [PATCH 2/2] ext4: fix __ext4_new_inode() journal credits calculation

[Theodore Ts'o]

On Wed, Jul 05, 2017 at 07:38:19PM -0700, Tahsin Erdogan wrote:
> ea_inode feature allows creating extended attributes that are up to
> 64k in size. Update __ext4_new_inode() to pick increased credit limits.
> 
> To avoid overallocating too many journal credits, update
> __ext4_xattr_set_credits() to make a distinction between xattr create
> vs update. This helps __ext4_new_inode() because all attributes are
> known to be new, so we can save credits that are normally needed to
> delete old values.
> 
> Also, have fscrypt specify its maximum context size so that we don't
> end up allocating credits for 64k size.
> 
> Signed-off-by: Tahsin Erdogan 

I've applied the following change to this patch in order to better
calculate the credits needed by ext4_new_inode.  The problem is that
it was estimating a worse case of 287 blocks for ext4_new_inode() on a
10MB file system using the default 1024 block size.  And on such a
file system, the typical size of the journal is 1024 blocks, and the
maximum number of credits that are allowed by a handle is 1024 / 4 =
256 blocks.  This cases a number of regression tests to blow up.

In reality the SElinux label and EVM/integrity xattrs are never going
to be 64k, so calculating the credits assuming that as the worst case
is not productive.  And normally the Posix ACL is never going to be a
worst case of 64k long, either...

- Ted

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 21a2538afcc2..507bfb3344d4 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -784,12 +784,38 @@ struct inode *__ext4_new_inode(handle_t *handle, struct 
inode *dir,
}
 
if (!handle && sbi->s_journal && !(i_flags & EXT4_EA_INODE_FL)) {
-   /*
-* 2 ea entries for ext4_init_acl(), 2 for ext4_init_security().
-*/
-   nblocks += 4 * __ext4_xattr_set_credits(sb, NULL /* inode */,
-   NULL /* block_bh */, XATTR_SIZE_MAX,
+#ifdef CONFIG_EXT4_FS_POSIX_ACL
+   struct posix_acl *p = get_acl(dir, ACL_TYPE_DEFAULT);
+
+   if (p) {
+   int acl_size = p->a_count * sizeof(ext4_acl_entry);
+
+   nblocks += (S_ISDIR(mode) ? 2 : 1) *
+   __ext4_xattr_set_credits(sb, NULL /* inode */,
+   NULL /* block_bh */, acl_size,
+   true /* is_create */);
+   posix_acl_release(p);
+   }
+#endif
+
+#ifdef CONFIG_SECURITY
+   {
+   int num_security_xattrs = 1;
+
+#ifdef CONFIG_INTEGRITY
+   num_security_xattrs++;
+#endif
+   /*
+* We assume that security xattrs are never
+* more than 1k.  In practice they are under
+* 128 bytes.
+*/
+   nblocks += num_security_xattrs *
+   __ext4_xattr_set_credits(sb, NULL /* inode */,
+   NULL /* block_bh */, 1024,
true /* is_create */);
+   }
+#endif
if (encrypt)
nblocks += __ext4_xattr_set_credits(sb,
NULL /* inode */, NULL /* block_bh */,

Re: [PATCH 2/2] ext4: fix __ext4_new_inode() journal credits calculation

[Theodore Ts'o]

On Wed, Jul 05, 2017 at 07:38:19PM -0700, Tahsin Erdogan wrote:
> ea_inode feature allows creating extended attributes that are up to
> 64k in size. Update __ext4_new_inode() to pick increased credit limits.
> 
> To avoid overallocating too many journal credits, update
> __ext4_xattr_set_credits() to make a distinction between xattr create
> vs update. This helps __ext4_new_inode() because all attributes are
> known to be new, so we can save credits that are normally needed to
> delete old values.
> 
> Also, have fscrypt specify its maximum context size so that we don't
> end up allocating credits for 64k size.
> 
> Signed-off-by: Tahsin Erdogan 

I've applied the following change to this patch in order to better
calculate the credits needed by ext4_new_inode.  The problem is that
it was estimating a worse case of 287 blocks for ext4_new_inode() on a
10MB file system using the default 1024 block size.  And on such a
file system, the typical size of the journal is 1024 blocks, and the
maximum number of credits that are allowed by a handle is 1024 / 4 =
256 blocks.  This cases a number of regression tests to blow up.

In reality the SElinux label and EVM/integrity xattrs are never going
to be 64k, so calculating the credits assuming that as the worst case
is not productive.  And normally the Posix ACL is never going to be a
worst case of 64k long, either...

- Ted

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 21a2538afcc2..507bfb3344d4 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -784,12 +784,38 @@ struct inode *__ext4_new_inode(handle_t *handle, struct 
inode *dir,
}
 
if (!handle && sbi->s_journal && !(i_flags & EXT4_EA_INODE_FL)) {
-   /*
-* 2 ea entries for ext4_init_acl(), 2 for ext4_init_security().
-*/
-   nblocks += 4 * __ext4_xattr_set_credits(sb, NULL /* inode */,
-   NULL /* block_bh */, XATTR_SIZE_MAX,
+#ifdef CONFIG_EXT4_FS_POSIX_ACL
+   struct posix_acl *p = get_acl(dir, ACL_TYPE_DEFAULT);
+
+   if (p) {
+   int acl_size = p->a_count * sizeof(ext4_acl_entry);
+
+   nblocks += (S_ISDIR(mode) ? 2 : 1) *
+   __ext4_xattr_set_credits(sb, NULL /* inode */,
+   NULL /* block_bh */, acl_size,
+   true /* is_create */);
+   posix_acl_release(p);
+   }
+#endif
+
+#ifdef CONFIG_SECURITY
+   {
+   int num_security_xattrs = 1;
+
+#ifdef CONFIG_INTEGRITY
+   num_security_xattrs++;
+#endif
+   /*
+* We assume that security xattrs are never
+* more than 1k.  In practice they are under
+* 128 bytes.
+*/
+   nblocks += num_security_xattrs *
+   __ext4_xattr_set_credits(sb, NULL /* inode */,
+   NULL /* block_bh */, 1024,
true /* is_create */);
+   }
+#endif
if (encrypt)
nblocks += __ext4_xattr_set_credits(sb,
NULL /* inode */, NULL /* block_bh */,

[PATCH] cisco: enic: Fic an error handling path in 'vnic_dev_init_devcmd2()'

[Christophe JAILLET]

if 'ioread32()' returns 0xFFF, we have to go through the error
handling path as done everywhere else in this function.

Move the 'err_free_wq' label to better match its name and its location
and add a new label 'err_disable_wq'.
Update the code accordingly.

Fixes: 373fb0873d43 ("enic: add devcmd2")
Signed-off-by: Christophe JAILLET 
---
 drivers/net/ethernet/cisco/enic/vnic_dev.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c 
b/drivers/net/ethernet/cisco/enic/vnic_dev.c
index 1841ad45d215..39bad67422dd 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
@@ -402,8 +402,8 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
fetch_index = ioread32(>devcmd2->wq.ctrl->fetch_index);
if (fetch_index == 0x) { /* check for hardware gone  */
vdev_err(vdev, "Fatal error in devcmd2 init - hardware surprise 
removal\n");
-
-   return -ENODEV;
+   err = -ENODEV;
+   goto err_free_wq;
}
 
enic_wq_init_start(>devcmd2->wq, 0, fetch_index, fetch_index, 0,
@@ -414,7 +414,7 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
err = vnic_dev_alloc_desc_ring(vdev, >devcmd2->results_ring,
   DEVCMD2_RING_SIZE, DEVCMD2_DESC_SIZE);
if (err)
-   goto err_free_wq;
+   goto err_disable_wq;
 
vdev->devcmd2->result = vdev->devcmd2->results_ring.descs;
vdev->devcmd2->cmd_ring = vdev->devcmd2->wq.ring.descs;
@@ -433,8 +433,9 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
 
 err_free_desc_ring:
vnic_dev_free_desc_ring(vdev, >devcmd2->results_ring);
-err_free_wq:
+err_disable_wq:
vnic_wq_disable(>devcmd2->wq);
+err_free_wq:
vnic_wq_free(>devcmd2->wq);
 err_free_devcmd2:
kfree(vdev->devcmd2);
-- 
2.11.0

[PATCH] cisco: enic: Fic an error handling path in 'vnic_dev_init_devcmd2()'

[Christophe JAILLET]

if 'ioread32()' returns 0xFFF, we have to go through the error
handling path as done everywhere else in this function.

Move the 'err_free_wq' label to better match its name and its location
and add a new label 'err_disable_wq'.
Update the code accordingly.

Fixes: 373fb0873d43 ("enic: add devcmd2")
Signed-off-by: Christophe JAILLET 
---
 drivers/net/ethernet/cisco/enic/vnic_dev.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/cisco/enic/vnic_dev.c 
b/drivers/net/ethernet/cisco/enic/vnic_dev.c
index 1841ad45d215..39bad67422dd 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_dev.c
+++ b/drivers/net/ethernet/cisco/enic/vnic_dev.c
@@ -402,8 +402,8 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
fetch_index = ioread32(>devcmd2->wq.ctrl->fetch_index);
if (fetch_index == 0x) { /* check for hardware gone  */
vdev_err(vdev, "Fatal error in devcmd2 init - hardware surprise 
removal\n");
-
-   return -ENODEV;
+   err = -ENODEV;
+   goto err_free_wq;
}
 
enic_wq_init_start(>devcmd2->wq, 0, fetch_index, fetch_index, 0,
@@ -414,7 +414,7 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
err = vnic_dev_alloc_desc_ring(vdev, >devcmd2->results_ring,
   DEVCMD2_RING_SIZE, DEVCMD2_DESC_SIZE);
if (err)
-   goto err_free_wq;
+   goto err_disable_wq;
 
vdev->devcmd2->result = vdev->devcmd2->results_ring.descs;
vdev->devcmd2->cmd_ring = vdev->devcmd2->wq.ring.descs;
@@ -433,8 +433,9 @@ static int vnic_dev_init_devcmd2(struct vnic_dev *vdev)
 
 err_free_desc_ring:
vnic_dev_free_desc_ring(vdev, >devcmd2->results_ring);
-err_free_wq:
+err_disable_wq:
vnic_wq_disable(>devcmd2->wq);
+err_free_wq:
vnic_wq_free(>devcmd2->wq);
 err_free_devcmd2:
kfree(vdev->devcmd2);
-- 
2.11.0

Re: [git pull] vfs.git pile 11 - iov_iter/hardening

[Linus Torvalds]

On Fri, Jul 7, 2017 at 5:29 PM, Al Viro  wrote:
>
> Trivial conflicts with libnvdimm; this stuff will get some
> followups, but again, that's for another series.

Gaah. Yeah, I guess I could have done the trivial ugly merge that just
took the new copy_from_iter_flushcache() as-is, and didn't match it up
with all the new iov_iter hardening.

Except I decided I don't want that, and want to do a proper merge instead.

Which made the trivial merge something that actually changed that
copy_from_iter_flushcache() logic, and maybe I screwed it up in the
process.

It builds, and it looks right to me, but you and Dan should really
check out the end result.

Particularly so for the CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE logic,
which is where this differs from the other cases, and which I changed
to make the #ifdef less noticeable.

I did try both a i386 and a x86-64 build of the iov_iter code, since
that should test both of those ARCH_HAS cases, but that was purely a
build test.

Also, the new __must_check warnings do trigger. At least for lustre. I
couldn't be arsed to try to fix those, since .. lustre. But I'm also
adding a couple of lustre people to the cc, just to make them aware of
it.. I'd really like the allmodconfig build to be clean.

Linus

Re: [git pull] vfs.git pile 11 - iov_iter/hardening

[Linus Torvalds]

On Fri, Jul 7, 2017 at 5:29 PM, Al Viro  wrote:
>
> Trivial conflicts with libnvdimm; this stuff will get some
> followups, but again, that's for another series.

Gaah. Yeah, I guess I could have done the trivial ugly merge that just
took the new copy_from_iter_flushcache() as-is, and didn't match it up
with all the new iov_iter hardening.

Except I decided I don't want that, and want to do a proper merge instead.

Which made the trivial merge something that actually changed that
copy_from_iter_flushcache() logic, and maybe I screwed it up in the
process.

It builds, and it looks right to me, but you and Dan should really
check out the end result.

Particularly so for the CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE logic,
which is where this differs from the other cases, and which I changed
to make the #ifdef less noticeable.

I did try both a i386 and a x86-64 build of the iov_iter code, since
that should test both of those ARCH_HAS cases, but that was purely a
build test.

Also, the new __must_check warnings do trigger. At least for lustre. I
couldn't be arsed to try to fix those, since .. lustre. But I'm also
adding a couple of lustre people to the cc, just to make them aware of
it.. I'd really like the allmodconfig build to be clean.

Linus

[RESEND PATCH v2 6/7] pwm: rockchip: Add rk3328 pwm support

[David Wu]

The rk3328 soc supports atomic update, we could lock the configuration
of period and duty at first, after unlock is configured, the period and
duty are effective at the same time.

If the polarity, period and duty need to be configured together,
the way for atomic update is "configure lock and old polarity" ->
"configure period and duty" -> "configure unlock and new polarity".

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
change in v2:
 - 3 different pwm_ops apply(),one for each IP revision.
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  1 +
 drivers/pwm/pwm-rockchip.c | 63 --
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 2350ef9..152c736 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -4,6 +4,7 @@ Required properties:
  - compatible: should be "rockchip,-pwm"
"rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
"rockchip,rk3288-pwm": found on RK3288 SoC
+   "rockchip,rk3328-pwm": found on RK3328 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
  - clocks: See ../clock/clock-bindings.txt
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 83703e1..838e8fc 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -29,6 +29,7 @@
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
 #define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
+#define PWM_LOCK_EN(1 << 6)
 #define PWM_LP_DISABLE (0 << 8)
 
 struct rockchip_pwm_chip {
@@ -124,13 +125,24 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  struct pwm_state *state, bool polarity)
+  struct pwm_state *state, bool polarity,
+  bool lock)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
u32 ctrl;
 
+   /*
+* Lock the period and duty of previous configuration, then
+* change the duty and period, that would not be effective.
+*/
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (lock) {
+   ctrl |= PWM_LOCK_EN;
+   writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
+   }
+
clk_rate = clk_get_rate(pc->clk);
 
/*
@@ -148,7 +160,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
 
-   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
if (polarity) {
ctrl &= ~PWM_POLARITY_MASK;
if (state->polarity == PWM_POLARITY_INVERSED)
@@ -156,6 +167,15 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
else
ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
}
+
+   /*
+* Unlock and set polarity at the same time,
+* the configuration of duty, period and polarity
+* would be effective together at next period.
+*/
+   if (lock)
+   ctrl &= ~PWM_LOCK_EN;
+
writel(ctrl, pc->base + pc->data->regs.ctrl);
 
return 0;
@@ -209,7 +229,7 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, false);
+   rockchip_pwm_config(chip, pwm, state, false, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
enable_conf);
@@ -236,7 +256,27 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, true);
+   rockchip_pwm_config(chip, pwm, state, true, false);
+   if (state->enabled != enabled)
+   ret = rockchip_pwm_enable(chip, pwm, state->enabled,
+ enable_conf);
+
+   return ret;
+}
+
+static int rockchip_pwm_apply_v3(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
+{
+   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
+ PWM_CONTINUOUS;
+   struct pwm_state curstate;
+   bool enabled;
+   int ret = 0;
+
+   pwm_get_state(pwm, );
+   enabled = 

[RESEND PATCH v2 6/7] pwm: rockchip: Add rk3328 pwm support

[David Wu]

The rk3328 soc supports atomic update, we could lock the configuration
of period and duty at first, after unlock is configured, the period and
duty are effective at the same time.

If the polarity, period and duty need to be configured together,
the way for atomic update is "configure lock and old polarity" ->
"configure period and duty" -> "configure unlock and new polarity".

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
change in v2:
 - 3 different pwm_ops apply(),one for each IP revision.
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  1 +
 drivers/pwm/pwm-rockchip.c | 63 --
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 2350ef9..152c736 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -4,6 +4,7 @@ Required properties:
  - compatible: should be "rockchip,-pwm"
"rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
"rockchip,rk3288-pwm": found on RK3288 SoC
+   "rockchip,rk3328-pwm": found on RK3328 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
  - clocks: See ../clock/clock-bindings.txt
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 83703e1..838e8fc 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -29,6 +29,7 @@
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
 #define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
+#define PWM_LOCK_EN(1 << 6)
 #define PWM_LP_DISABLE (0 << 8)
 
 struct rockchip_pwm_chip {
@@ -124,13 +125,24 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  struct pwm_state *state, bool polarity)
+  struct pwm_state *state, bool polarity,
+  bool lock)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
u32 ctrl;
 
+   /*
+* Lock the period and duty of previous configuration, then
+* change the duty and period, that would not be effective.
+*/
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (lock) {
+   ctrl |= PWM_LOCK_EN;
+   writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
+   }
+
clk_rate = clk_get_rate(pc->clk);
 
/*
@@ -148,7 +160,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
 
-   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
if (polarity) {
ctrl &= ~PWM_POLARITY_MASK;
if (state->polarity == PWM_POLARITY_INVERSED)
@@ -156,6 +167,15 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
else
ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
}
+
+   /*
+* Unlock and set polarity at the same time,
+* the configuration of duty, period and polarity
+* would be effective together at next period.
+*/
+   if (lock)
+   ctrl &= ~PWM_LOCK_EN;
+
writel(ctrl, pc->base + pc->data->regs.ctrl);
 
return 0;
@@ -209,7 +229,7 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, false);
+   rockchip_pwm_config(chip, pwm, state, false, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
enable_conf);
@@ -236,7 +256,27 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, true);
+   rockchip_pwm_config(chip, pwm, state, true, false);
+   if (state->enabled != enabled)
+   ret = rockchip_pwm_enable(chip, pwm, state->enabled,
+ enable_conf);
+
+   return ret;
+}
+
+static int rockchip_pwm_apply_v3(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
+{
+   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
+ PWM_CONTINUOUS;
+   struct pwm_state curstate;
+   bool enabled;
+   int ret = 0;
+
+   pwm_get_state(pwm, );
+   enabled = curstate.enabled;
+
+   

[PATCH v2 7/7] arm64: dts: rockchip: Add pwm nodes for rk3328

[David Wu]

There are 4 pwm channels built in rk3328 soc, need to configure
the both APB clock and bus clock.

Signed-off-by: David Wu 
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 
 1 file changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 29b3800..46f0847 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -310,6 +310,51 @@
interrupts = ;
};
 
+   pwm0: pwm@ff1b {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm1: pwm@ff1b0010 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0010 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm2: pwm@ff1b0020 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0020 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm3: pwm@ff1b0030 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0030 0x0 0x10>;
+   interrupts = ;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
saradc: adc@ff28 {
compatible = "rockchip,rk3328-saradc", "rockchip,rk3399-saradc";
reg = <0x0 0xff28 0x0 0x100>;
-- 
1.9.1

[PATCH v2 7/7] arm64: dts: rockchip: Add pwm nodes for rk3328

[David Wu]

There are 4 pwm channels built in rk3328 soc, need to configure
the both APB clock and bus clock.

Signed-off-by: David Wu 
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 
 1 file changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 29b3800..46f0847 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -310,6 +310,51 @@
interrupts = ;
};
 
+   pwm0: pwm@ff1b {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm1: pwm@ff1b0010 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0010 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm2: pwm@ff1b0020 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0020 0x0 0x10>;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
+   pwm3: pwm@ff1b0030 {
+   compatible = "rockchip,rk3328-pwm";
+   reg = <0x0 0xff1b0030 0x0 0x10>;
+   interrupts = ;
+   #pwm-cells = <3>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pin>;
+   clocks = < SCLK_PWM>, < PCLK_PWM>;
+   clock-names = "pwm", "pclk";
+   status = "disabled";
+   };
+
saradc: adc@ff28 {
compatible = "rockchip,rk3328-saradc", "rockchip,rk3399-saradc";
reg = <0x0 0xff28 0x0 0x100>;
-- 
1.9.1

[PATCH v2 6/7] pwm: rockchip: Add rk3328 pwm support

[David Wu]

The rk3328 soc supports atomic update, we could lock the configuration
of period and duty at first, after unlock is configured, the period and
duty are effective at the same time.

If the polarity, period and duty need to be configured together,
the way for atomic update is "configure lock and old polarity" ->
"configure period and duty" -> "configure unlock and new polarity".

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
change in v2:
 - 3 different pwm_ops apply(),one for each IP revision.
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  1 +
 drivers/pwm/pwm-rockchip.c | 63 --
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 2350ef9..152c736 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -4,6 +4,7 @@ Required properties:
  - compatible: should be "rockchip,-pwm"
"rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
"rockchip,rk3288-pwm": found on RK3288 SoC
+   "rockchip,rk3328-pwm": found on RK3328 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
  - clocks: See ../clock/clock-bindings.txt
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 83703e1..838e8fc 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -29,6 +29,7 @@
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
 #define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
+#define PWM_LOCK_EN(1 << 6)
 #define PWM_LP_DISABLE (0 << 8)
 
 struct rockchip_pwm_chip {
@@ -124,13 +125,24 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  struct pwm_state *state, bool polarity)
+  struct pwm_state *state, bool polarity,
+  bool lock)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
u32 ctrl;
 
+   /*
+* Lock the period and duty of previous configuration, then
+* change the duty and period, that would not be effective.
+*/
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (lock) {
+   ctrl |= PWM_LOCK_EN;
+   writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
+   }
+
clk_rate = clk_get_rate(pc->clk);
 
/*
@@ -148,7 +160,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
 
-   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
if (polarity) {
ctrl &= ~PWM_POLARITY_MASK;
if (state->polarity == PWM_POLARITY_INVERSED)
@@ -156,6 +167,15 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
else
ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
}
+
+   /*
+* Unlock and set polarity at the same time,
+* the configuration of duty, period and polarity
+* would be effective together at next period.
+*/
+   if (lock)
+   ctrl &= ~PWM_LOCK_EN;
+
writel(ctrl, pc->base + pc->data->regs.ctrl);
 
return 0;
@@ -209,7 +229,7 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, false);
+   rockchip_pwm_config(chip, pwm, state, false, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
enable_conf);
@@ -236,7 +256,27 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, true);
+   rockchip_pwm_config(chip, pwm, state, true, false);
+   if (state->enabled != enabled)
+   ret = rockchip_pwm_enable(chip, pwm, state->enabled,
+ enable_conf);
+
+   return ret;
+}
+
+static int rockchip_pwm_apply_v3(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
+{
+   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
+ PWM_CONTINUOUS;
+   struct pwm_state curstate;
+   bool enabled;
+   int ret = 0;
+
+   pwm_get_state(pwm, );
+   enabled = 

[PATCH v2 6/7] pwm: rockchip: Add rk3328 pwm support

[David Wu]

The rk3328 soc supports atomic update, we could lock the configuration
of period and duty at first, after unlock is configured, the period and
duty are effective at the same time.

If the polarity, period and duty need to be configured together,
the way for atomic update is "configure lock and old polarity" ->
"configure period and duty" -> "configure unlock and new polarity".

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
change in v2:
 - 3 different pwm_ops apply(),one for each IP revision.
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  1 +
 drivers/pwm/pwm-rockchip.c | 63 --
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index 2350ef9..152c736 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -4,6 +4,7 @@ Required properties:
  - compatible: should be "rockchip,-pwm"
"rockchip,rk2928-pwm": found on RK29XX,RK3066 and RK3188 SoCs
"rockchip,rk3288-pwm": found on RK3288 SoC
+   "rockchip,rk3328-pwm": found on RK3328 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
  - clocks: See ../clock/clock-bindings.txt
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 83703e1..838e8fc 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -29,6 +29,7 @@
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
 #define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
+#define PWM_LOCK_EN(1 << 6)
 #define PWM_LP_DISABLE (0 << 8)
 
 struct rockchip_pwm_chip {
@@ -124,13 +125,24 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  struct pwm_state *state, bool polarity)
+  struct pwm_state *state, bool polarity,
+  bool lock)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
u32 ctrl;
 
+   /*
+* Lock the period and duty of previous configuration, then
+* change the duty and period, that would not be effective.
+*/
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (lock) {
+   ctrl |= PWM_LOCK_EN;
+   writel_relaxed(ctrl, pc->base + pc->data->regs.ctrl);
+   }
+
clk_rate = clk_get_rate(pc->clk);
 
/*
@@ -148,7 +160,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
 
-   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
if (polarity) {
ctrl &= ~PWM_POLARITY_MASK;
if (state->polarity == PWM_POLARITY_INVERSED)
@@ -156,6 +167,15 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
else
ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
}
+
+   /*
+* Unlock and set polarity at the same time,
+* the configuration of duty, period and polarity
+* would be effective together at next period.
+*/
+   if (lock)
+   ctrl &= ~PWM_LOCK_EN;
+
writel(ctrl, pc->base + pc->data->regs.ctrl);
 
return 0;
@@ -209,7 +229,7 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, false);
+   rockchip_pwm_config(chip, pwm, state, false, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
enable_conf);
@@ -236,7 +256,27 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state, true);
+   rockchip_pwm_config(chip, pwm, state, true, false);
+   if (state->enabled != enabled)
+   ret = rockchip_pwm_enable(chip, pwm, state->enabled,
+ enable_conf);
+
+   return ret;
+}
+
+static int rockchip_pwm_apply_v3(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
+{
+   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
+ PWM_CONTINUOUS;
+   struct pwm_state curstate;
+   bool enabled;
+   int ret = 0;
+
+   pwm_get_state(pwm, );
+   enabled = curstate.enabled;
+
+   

[PATCH v2 5/7] pwm: rockchip: Move the configuration of polarity from rockchip_pwm_set_enable() to rockchip_pwm_config()

[David Wu]

It is usually possible to configure the polarity, cycle and duty all at once,
so that the polarity and cycle and duty should be binding together. Move it
into rockchip_pwm_config(), as well as prepared for the next atomic update
commit.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 12e7b4a8..83703e1 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -27,6 +27,7 @@
 #define PWM_DUTY_NEGATIVE  (0 << 3)
 #define PWM_INACTIVE_NEGATIVE  (0 << 4)
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
+#define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
 #define PWM_LP_DISABLE (0 << 8)
 
@@ -122,12 +123,13 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
 }
 
-static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  int duty_ns, int period_ns)
+static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+  struct pwm_state *state, bool polarity)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
+   u32 ctrl;
 
clk_rate = clk_get_rate(pc->clk);
 
@@ -136,21 +138,32 @@ static void rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 * bits, every possible input period can be obtained using the
 * default prescaler value for all practical clock rate values.
 */
-   div = clk_rate * period_ns;
+   div = clk_rate * state->period;
period = DIV_ROUND_CLOSEST_ULL(div,
   pc->data->prescaler * NSEC_PER_SEC);
 
-   div = clk_rate * duty_ns;
+   div = clk_rate * state->duty_cycle;
duty = DIV_ROUND_CLOSEST_ULL(div, pc->data->prescaler * NSEC_PER_SEC);
 
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
+
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (polarity) {
+   ctrl &= ~PWM_POLARITY_MASK;
+   if (state->polarity == PWM_POLARITY_INVERSED)
+   ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
+   else
+   ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
+   }
+   writel(ctrl, pc->base + pc->data->regs.ctrl);
+
+   return 0;
 }
 
 static int rockchip_pwm_enable(struct pwm_chip *chip,
 struct pwm_device *pwm,
 bool enable,
-enum pwm_polarity polarity,
 u32 enable_conf)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
@@ -163,15 +176,6 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return ret;
}
 
-   if (pc->data->supports_polarity) {
-   if (polarity == PWM_POLARITY_INVERSED)
-   enable_conf |= PWM_DUTY_NEGATIVE |
-  PWM_INACTIVE_POSITIVE;
-   else
-   enable_conf |= PWM_DUTY_POSITIVE |
-  PWM_INACTIVE_NEGATIVE;
-   }
-
val = readl_relaxed(pc->base + pc->data->regs.ctrl);
 
if (enable)
@@ -199,17 +203,16 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = curstate.enabled;
 
if (state->polarity != curstate.polarity && enabled) {
-   ret = rockchip_pwm_enable(chip, pwm, false, state->polarity,
- enable_conf);
+   ret = rockchip_pwm_enable(chip, pwm, false, enable_conf);
if (ret)
return ret;
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
+   rockchip_pwm_config(chip, pwm, state, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
-   state->polarity, enable_conf);
+   enable_conf);
 
return ret;
 }
@@ -227,17 +230,16 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = curstate.enabled;
 
if (state->polarity != curstate.polarity && enabled) {
-   ret = rockchip_pwm_enable(chip, pwm, false, state->polarity,
- enable_conf);
+   ret = rockchip_pwm_enable(chip, pwm, false, enable_conf);
if (ret)
return ret;
enabled = false;
}
 
-   

[PATCH v2 5/7] pwm: rockchip: Move the configuration of polarity from rockchip_pwm_set_enable() to rockchip_pwm_config()

[David Wu]

It is usually possible to configure the polarity, cycle and duty all at once,
so that the polarity and cycle and duty should be binding together. Move it
into rockchip_pwm_config(), as well as prepared for the next atomic update
commit.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 12e7b4a8..83703e1 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -27,6 +27,7 @@
 #define PWM_DUTY_NEGATIVE  (0 << 3)
 #define PWM_INACTIVE_NEGATIVE  (0 << 4)
 #define PWM_INACTIVE_POSITIVE  (1 << 4)
+#define PWM_POLARITY_MASK  (PWM_DUTY_POSITIVE | PWM_INACTIVE_POSITIVE)
 #define PWM_OUTPUT_LEFT(0 << 5)
 #define PWM_LP_DISABLE (0 << 8)
 
@@ -122,12 +123,13 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
 }
 
-static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  int duty_ns, int period_ns)
+static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+  struct pwm_state *state, bool polarity)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
unsigned long period, duty;
u64 clk_rate, div;
+   u32 ctrl;
 
clk_rate = clk_get_rate(pc->clk);
 
@@ -136,21 +138,32 @@ static void rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 * bits, every possible input period can be obtained using the
 * default prescaler value for all practical clock rate values.
 */
-   div = clk_rate * period_ns;
+   div = clk_rate * state->period;
period = DIV_ROUND_CLOSEST_ULL(div,
   pc->data->prescaler * NSEC_PER_SEC);
 
-   div = clk_rate * duty_ns;
+   div = clk_rate * state->duty_cycle;
duty = DIV_ROUND_CLOSEST_ULL(div, pc->data->prescaler * NSEC_PER_SEC);
 
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
+
+   ctrl = readl_relaxed(pc->base + pc->data->regs.ctrl);
+   if (polarity) {
+   ctrl &= ~PWM_POLARITY_MASK;
+   if (state->polarity == PWM_POLARITY_INVERSED)
+   ctrl |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
+   else
+   ctrl |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
+   }
+   writel(ctrl, pc->base + pc->data->regs.ctrl);
+
+   return 0;
 }
 
 static int rockchip_pwm_enable(struct pwm_chip *chip,
 struct pwm_device *pwm,
 bool enable,
-enum pwm_polarity polarity,
 u32 enable_conf)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
@@ -163,15 +176,6 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return ret;
}
 
-   if (pc->data->supports_polarity) {
-   if (polarity == PWM_POLARITY_INVERSED)
-   enable_conf |= PWM_DUTY_NEGATIVE |
-  PWM_INACTIVE_POSITIVE;
-   else
-   enable_conf |= PWM_DUTY_POSITIVE |
-  PWM_INACTIVE_NEGATIVE;
-   }
-
val = readl_relaxed(pc->base + pc->data->regs.ctrl);
 
if (enable)
@@ -199,17 +203,16 @@ static int rockchip_pwm_apply_v1(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = curstate.enabled;
 
if (state->polarity != curstate.polarity && enabled) {
-   ret = rockchip_pwm_enable(chip, pwm, false, state->polarity,
- enable_conf);
+   ret = rockchip_pwm_enable(chip, pwm, false, enable_conf);
if (ret)
return ret;
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
+   rockchip_pwm_config(chip, pwm, state, false);
if (state->enabled != enabled)
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
-   state->polarity, enable_conf);
+   enable_conf);
 
return ret;
 }
@@ -227,17 +230,16 @@ static int rockchip_pwm_apply_v2(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = curstate.enabled;
 
if (state->polarity != curstate.polarity && enabled) {
-   ret = rockchip_pwm_enable(chip, pwm, false, state->polarity,
- enable_conf);
+   ret = rockchip_pwm_enable(chip, pwm, false, enable_conf);
if (ret)
return ret;
enabled = false;
}
 
-   rockchip_pwm_config(chip, pwm, 

[PATCH v2 4/7] pwm: rockchip: Use pwm_apply instead of the pwm_enable

[David Wu]

Drop the custom hook of pwm_enable and implement
pwm_apply_v1 and pwm_apply_v2 instead.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 141 +
 1 file changed, 77 insertions(+), 64 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 85f9515..12e7b4a8 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -51,11 +51,10 @@ struct rockchip_pwm_data {
bool supports_polarity;
const struct pwm_ops *ops;
 
-   void (*set_enable)(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity);
void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
  struct pwm_state *state);
+   int (*pwm_apply)(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state);
 };
 
 static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip 
*c)
@@ -63,24 +62,6 @@ static inline struct rockchip_pwm_chip 
*to_rockchip_pwm_chip(struct pwm_chip *c)
return container_of(c, struct rockchip_pwm_chip, chip);
 }
 
-static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity)
-{
-   struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
-   u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
-   u32 val;
-
-   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
-
-   if (enable)
-   val |= enable_conf;
-   else
-   val &= ~enable_conf;
-
-   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
-}
-
 static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
  struct pwm_device *pwm,
  struct pwm_state *state)
@@ -94,30 +75,6 @@ static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
state->enabled = true;
 }
 
-static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity)
-{
-   struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
-   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
- PWM_CONTINUOUS;
-   u32 val;
-
-   if (polarity == PWM_POLARITY_INVERSED)
-   enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
-   else
-   enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
-
-   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
-
-   if (enable)
-   val |= enable_conf;
-   else
-   val &= ~enable_conf;
-
-   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
-}
-
 static void rockchip_pwm_get_state_v2(struct pwm_chip *chip,
  struct pwm_device *pwm,
  struct pwm_state *state)
@@ -193,10 +150,12 @@ static void rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 static int rockchip_pwm_enable(struct pwm_chip *chip,
 struct pwm_device *pwm,
 bool enable,
-enum pwm_polarity polarity)
+enum pwm_polarity polarity,
+u32 enable_conf)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
int ret;
+   u32 val;
 
if (enable) {
ret = clk_enable(pc->clk);
@@ -204,7 +163,23 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return ret;
}
 
-   pc->data->set_enable(chip, pwm, enable, polarity);
+   if (pc->data->supports_polarity) {
+   if (polarity == PWM_POLARITY_INVERSED)
+   enable_conf |= PWM_DUTY_NEGATIVE |
+  PWM_INACTIVE_POSITIVE;
+   else
+   enable_conf |= PWM_DUTY_POSITIVE |
+  PWM_INACTIVE_NEGATIVE;
+   }
+
+   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
+
+   if (enable)
+   val |= enable_conf;
+   else
+   val &= ~enable_conf;
+
+   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
 
if (!enable)
clk_disable(pc->clk);
@@ -212,37 +187,75 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return 0;
 }
 
-static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+static int rockchip_pwm_apply_v1(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
 {
-   struct 

[PATCH v2 0/7] Add rk3328 pwm support

[David Wu]

There are two features of rk3328 pwm module.
 - PWM APB and function clocks are different.
 - Add pwm atomic hardware update

David Wu (7):
  pwm: rockchip: Add APB and function both clocks support
  pwm: rockchip: Remove the judge from return value of pwm_config
  pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops
  pwm: rockchip: Use pwm_apply instead of the pwm_enable
  pwm: rockchip: Move the configuration of polarity from
rockchip_pwm_set_enable() to rockchip_pwm_config()
  pwm: rockchip: Add rk3328 pwm support
  arm64: dts: rockchip: Add pwm nodes for rk3328

 .../devicetree/bindings/pwm/pwm-rockchip.txt   |   9 +-
 arch/arm64/boot/dts/rockchip/rk3328.dtsi   |  45 
 drivers/pwm/pwm-rockchip.c | 275 ++---
 3 files changed, 236 insertions(+), 93 deletions(-)

-- 
1.9.1

[PATCH v2 4/7] pwm: rockchip: Use pwm_apply instead of the pwm_enable

[David Wu]

Drop the custom hook of pwm_enable and implement
pwm_apply_v1 and pwm_apply_v2 instead.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 141 +
 1 file changed, 77 insertions(+), 64 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 85f9515..12e7b4a8 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -51,11 +51,10 @@ struct rockchip_pwm_data {
bool supports_polarity;
const struct pwm_ops *ops;
 
-   void (*set_enable)(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity);
void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,
  struct pwm_state *state);
+   int (*pwm_apply)(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state);
 };
 
 static inline struct rockchip_pwm_chip *to_rockchip_pwm_chip(struct pwm_chip 
*c)
@@ -63,24 +62,6 @@ static inline struct rockchip_pwm_chip 
*to_rockchip_pwm_chip(struct pwm_chip *c)
return container_of(c, struct rockchip_pwm_chip, chip);
 }
 
-static void rockchip_pwm_set_enable_v1(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity)
-{
-   struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
-   u32 enable_conf = PWM_CTRL_OUTPUT_EN | PWM_CTRL_TIMER_EN;
-   u32 val;
-
-   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
-
-   if (enable)
-   val |= enable_conf;
-   else
-   val &= ~enable_conf;
-
-   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
-}
-
 static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
  struct pwm_device *pwm,
  struct pwm_state *state)
@@ -94,30 +75,6 @@ static void rockchip_pwm_get_state_v1(struct pwm_chip *chip,
state->enabled = true;
 }
 
-static void rockchip_pwm_set_enable_v2(struct pwm_chip *chip,
-  struct pwm_device *pwm, bool enable,
-  enum pwm_polarity polarity)
-{
-   struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
-   u32 enable_conf = PWM_OUTPUT_LEFT | PWM_LP_DISABLE | PWM_ENABLE |
- PWM_CONTINUOUS;
-   u32 val;
-
-   if (polarity == PWM_POLARITY_INVERSED)
-   enable_conf |= PWM_DUTY_NEGATIVE | PWM_INACTIVE_POSITIVE;
-   else
-   enable_conf |= PWM_DUTY_POSITIVE | PWM_INACTIVE_NEGATIVE;
-
-   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
-
-   if (enable)
-   val |= enable_conf;
-   else
-   val &= ~enable_conf;
-
-   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
-}
-
 static void rockchip_pwm_get_state_v2(struct pwm_chip *chip,
  struct pwm_device *pwm,
  struct pwm_state *state)
@@ -193,10 +150,12 @@ static void rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 static int rockchip_pwm_enable(struct pwm_chip *chip,
 struct pwm_device *pwm,
 bool enable,
-enum pwm_polarity polarity)
+enum pwm_polarity polarity,
+u32 enable_conf)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
int ret;
+   u32 val;
 
if (enable) {
ret = clk_enable(pc->clk);
@@ -204,7 +163,23 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return ret;
}
 
-   pc->data->set_enable(chip, pwm, enable, polarity);
+   if (pc->data->supports_polarity) {
+   if (polarity == PWM_POLARITY_INVERSED)
+   enable_conf |= PWM_DUTY_NEGATIVE |
+  PWM_INACTIVE_POSITIVE;
+   else
+   enable_conf |= PWM_DUTY_POSITIVE |
+  PWM_INACTIVE_NEGATIVE;
+   }
+
+   val = readl_relaxed(pc->base + pc->data->regs.ctrl);
+
+   if (enable)
+   val |= enable_conf;
+   else
+   val &= ~enable_conf;
+
+   writel_relaxed(val, pc->base + pc->data->regs.ctrl);
 
if (!enable)
clk_disable(pc->clk);
@@ -212,37 +187,75 @@ static int rockchip_pwm_enable(struct pwm_chip *chip,
return 0;
 }
 
-static int rockchip_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
+static int rockchip_pwm_apply_v1(struct pwm_chip *chip, struct pwm_device *pwm,
+struct pwm_state *state)
 {
-   struct rockchip_pwm_chip *pc = 

[PATCH v2 0/7] Add rk3328 pwm support

[David Wu]

There are two features of rk3328 pwm module.
 - PWM APB and function clocks are different.
 - Add pwm atomic hardware update

David Wu (7):
  pwm: rockchip: Add APB and function both clocks support
  pwm: rockchip: Remove the judge from return value of pwm_config
  pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops
  pwm: rockchip: Use pwm_apply instead of the pwm_enable
  pwm: rockchip: Move the configuration of polarity from
rockchip_pwm_set_enable() to rockchip_pwm_config()
  pwm: rockchip: Add rk3328 pwm support
  arm64: dts: rockchip: Add pwm nodes for rk3328

 .../devicetree/bindings/pwm/pwm-rockchip.txt   |   9 +-
 arch/arm64/boot/dts/rockchip/rk3328.dtsi   |  45 
 drivers/pwm/pwm-rockchip.c | 275 ++---
 3 files changed, 236 insertions(+), 93 deletions(-)

-- 
1.9.1

[PATCH v2 2/7] pwm: rockchip: Remove the judge from return value of pwm_config

[David Wu]

It seems the rockchip_pwm_config always returns the result 0,
so remove the judge.

Signed-off-by: David Wu 
Acked-by: Boris Brezillon 
---
 drivers/pwm/pwm-rockchip.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 617824c..cd45f17 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -165,7 +165,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
 }
 
-static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
   int duty_ns, int period_ns)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
@@ -188,8 +188,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
-
-   return 0;
 }
 
 static int rockchip_pwm_enable(struct pwm_chip *chip,
@@ -236,13 +234,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   ret = rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
-   if (ret) {
-   if (enabled != curstate.enabled)
-   rockchip_pwm_enable(chip, pwm, !enabled,
- state->polarity);
-   goto out;
-   }
+   rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
 
if (state->enabled != enabled) {
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
-- 
1.9.1

[PATCH v2 1/7] pwm: rockchip: Add APB and function both clocks support

[David Wu]

New PWM module provides two individual clocks for APB clock
and function clock.

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  8 +++-
 drivers/pwm/pwm-rockchip.c | 53 ++
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index b8be3d0..2350ef9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -6,7 +6,13 @@ Required properties:
"rockchip,rk3288-pwm": found on RK3288 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
- - clocks: phandle and clock specifier of the PWM reference clock
+ - clocks: See ../clock/clock-bindings.txt
+   - For older hardware (rk2928, rk3066, rk3188, rk3228, rk3288, rk3399):
+ - There is one clock that's used both to derive the functional clock
+   for the device and as the bus clock.
+   - For newer hardware (rk3328 and future socs): specified by name
+ - "pwm": This is used to derive the functional clock.
+ - "pclk": This is the APB bus clock.
  - #pwm-cells: must be 2 (rk2928) or 3 (rk3288). See pwm.txt in this directory
for a description of the cell format.
 
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 744d561..617824c 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -33,6 +33,7 @@
 struct rockchip_pwm_chip {
struct pwm_chip chip;
struct clk *clk;
+   struct clk *pclk;
const struct rockchip_pwm_data *data;
void __iomem *base;
 };
@@ -145,7 +146,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
u64 tmp;
int ret;
 
-   ret = clk_enable(pc->clk);
+   ret = clk_enable(pc->pclk);
if (ret)
return;
 
@@ -161,7 +162,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 
pc->data->get_state(chip, pwm, state);
 
-   clk_disable(pc->clk);
+   clk_disable(pc->pclk);
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -224,7 +225,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
pwm_get_state(pwm, );
enabled = curstate.enabled;
 
-   ret = clk_enable(pc->clk);
+   ret = clk_enable(pc->pclk);
if (ret)
return ret;
 
@@ -257,7 +258,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
rockchip_pwm_get_state(chip, pwm, state);
 
 out:
-   clk_disable(pc->clk);
+   clk_disable(pc->pclk);
 
return ret;
 }
@@ -328,7 +329,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
const struct of_device_id *id;
struct rockchip_pwm_chip *pc;
struct resource *r;
-   int ret;
+   int ret, count;
 
id = of_match_device(rockchip_pwm_dt_ids, >dev);
if (!id)
@@ -343,13 +344,38 @@ static int rockchip_pwm_probe(struct platform_device 
*pdev)
if (IS_ERR(pc->base))
return PTR_ERR(pc->base);
 
-   pc->clk = devm_clk_get(>dev, NULL);
-   if (IS_ERR(pc->clk))
-   return PTR_ERR(pc->clk);
+   pc->clk = devm_clk_get(>dev, "pwm");
+   count = of_property_count_strings(pdev->dev.of_node, "clock-names");
+   if (count == 2)
+   pc->pclk = devm_clk_get(>dev, "pclk");
+   else
+   pc->pclk = pc->clk;
+
+   if (IS_ERR(pc->clk)) {
+   ret = PTR_ERR(pc->clk);
+   if (ret != -EPROBE_DEFER)
+   dev_err(>dev, "Can't get bus clk: %d\n", ret);
+   return ret;
+   }
+
+   if (IS_ERR(pc->pclk)) {
+   ret = PTR_ERR(pc->pclk);
+   if (ret != -EPROBE_DEFER)
+   dev_err(>dev, "Can't get APB clk: %d\n", ret);
+   return ret;
+   }
 
ret = clk_prepare_enable(pc->clk);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "Can't prepare enable bus clk: %d\n", ret);
return ret;
+   }
+
+   ret = clk_prepare(pc->pclk);
+   if (ret) {
+   dev_err(>dev, "Can't prepare APB clk: %d\n", ret);
+   goto err_clk;
+   }
 
platform_set_drvdata(pdev, pc);
 
@@ -368,12 +394,20 @@ static int rockchip_pwm_probe(struct platform_device 
*pdev)
if (ret < 0) {
clk_unprepare(pc->clk);
dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
+   goto err_pclk;
}
 
/* Keep the PWM clk enabled if the PWM appears to be up and running. */
if (!pwm_is_enabled(pc->chip.pwms))
clk_disable(pc->clk);
 
+   return 0;
+
+err_pclk:
+  

[PATCH v2 3/7] pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops

[David Wu]

The rockchip_pwm_ops_v1 and rockchip_pwm_ops_v2 ops are the same
struct members, remove one of them.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index cd45f17..85f9515 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -255,13 +255,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, 
struct pwm_device *pwm,
return ret;
 }
 
-static const struct pwm_ops rockchip_pwm_ops_v1 = {
-   .get_state = rockchip_pwm_get_state,
-   .apply = rockchip_pwm_apply,
-   .owner = THIS_MODULE,
-};
-
-static const struct pwm_ops rockchip_pwm_ops_v2 = {
+static const struct pwm_ops rockchip_pwm_ops = {
.get_state = rockchip_pwm_get_state,
.apply = rockchip_pwm_apply,
.owner = THIS_MODULE,
@@ -275,7 +269,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
.ctrl = 0x0c,
},
.prescaler = 2,
-   .ops = _pwm_ops_v1,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v1,
.get_state = rockchip_pwm_get_state_v1,
 };
@@ -289,7 +283,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
},
.prescaler = 1,
.supports_polarity = true,
-   .ops = _pwm_ops_v2,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
 };
@@ -303,7 +297,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
},
.prescaler = 1,
.supports_polarity = true,
-   .ops = _pwm_ops_v2,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
 };
-- 
1.9.1

[PATCH v2 2/7] pwm: rockchip: Remove the judge from return value of pwm_config

[David Wu]

It seems the rockchip_pwm_config always returns the result 0,
so remove the judge.

Signed-off-by: David Wu 
Acked-by: Boris Brezillon 
---
 drivers/pwm/pwm-rockchip.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 617824c..cd45f17 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -165,7 +165,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
clk_disable(pc->pclk);
 }
 
-static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+static void rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
   int duty_ns, int period_ns)
 {
struct rockchip_pwm_chip *pc = to_rockchip_pwm_chip(chip);
@@ -188,8 +188,6 @@ static int rockchip_pwm_config(struct pwm_chip *chip, 
struct pwm_device *pwm,
 
writel(period, pc->base + pc->data->regs.period);
writel(duty, pc->base + pc->data->regs.duty);
-
-   return 0;
 }
 
 static int rockchip_pwm_enable(struct pwm_chip *chip,
@@ -236,13 +234,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, 
struct pwm_device *pwm,
enabled = false;
}
 
-   ret = rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
-   if (ret) {
-   if (enabled != curstate.enabled)
-   rockchip_pwm_enable(chip, pwm, !enabled,
- state->polarity);
-   goto out;
-   }
+   rockchip_pwm_config(chip, pwm, state->duty_cycle, state->period);
 
if (state->enabled != enabled) {
ret = rockchip_pwm_enable(chip, pwm, state->enabled,
-- 
1.9.1

[PATCH v2 1/7] pwm: rockchip: Add APB and function both clocks support

[David Wu]

New PWM module provides two individual clocks for APB clock
and function clock.

Signed-off-by: David Wu 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/pwm/pwm-rockchip.txt   |  8 +++-
 drivers/pwm/pwm-rockchip.c | 53 ++
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt 
b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
index b8be3d0..2350ef9 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm-rockchip.txt
@@ -6,7 +6,13 @@ Required properties:
"rockchip,rk3288-pwm": found on RK3288 SoC
"rockchip,vop-pwm": found integrated in VOP on RK3288 SoC
  - reg: physical base address and length of the controller's registers
- - clocks: phandle and clock specifier of the PWM reference clock
+ - clocks: See ../clock/clock-bindings.txt
+   - For older hardware (rk2928, rk3066, rk3188, rk3228, rk3288, rk3399):
+ - There is one clock that's used both to derive the functional clock
+   for the device and as the bus clock.
+   - For newer hardware (rk3328 and future socs): specified by name
+ - "pwm": This is used to derive the functional clock.
+ - "pclk": This is the APB bus clock.
  - #pwm-cells: must be 2 (rk2928) or 3 (rk3288). See pwm.txt in this directory
for a description of the cell format.
 
diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index 744d561..617824c 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -33,6 +33,7 @@
 struct rockchip_pwm_chip {
struct pwm_chip chip;
struct clk *clk;
+   struct clk *pclk;
const struct rockchip_pwm_data *data;
void __iomem *base;
 };
@@ -145,7 +146,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
u64 tmp;
int ret;
 
-   ret = clk_enable(pc->clk);
+   ret = clk_enable(pc->pclk);
if (ret)
return;
 
@@ -161,7 +162,7 @@ static void rockchip_pwm_get_state(struct pwm_chip *chip,
 
pc->data->get_state(chip, pwm, state);
 
-   clk_disable(pc->clk);
+   clk_disable(pc->pclk);
 }
 
 static int rockchip_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -224,7 +225,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
pwm_get_state(pwm, );
enabled = curstate.enabled;
 
-   ret = clk_enable(pc->clk);
+   ret = clk_enable(pc->pclk);
if (ret)
return ret;
 
@@ -257,7 +258,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
rockchip_pwm_get_state(chip, pwm, state);
 
 out:
-   clk_disable(pc->clk);
+   clk_disable(pc->pclk);
 
return ret;
 }
@@ -328,7 +329,7 @@ static int rockchip_pwm_probe(struct platform_device *pdev)
const struct of_device_id *id;
struct rockchip_pwm_chip *pc;
struct resource *r;
-   int ret;
+   int ret, count;
 
id = of_match_device(rockchip_pwm_dt_ids, >dev);
if (!id)
@@ -343,13 +344,38 @@ static int rockchip_pwm_probe(struct platform_device 
*pdev)
if (IS_ERR(pc->base))
return PTR_ERR(pc->base);
 
-   pc->clk = devm_clk_get(>dev, NULL);
-   if (IS_ERR(pc->clk))
-   return PTR_ERR(pc->clk);
+   pc->clk = devm_clk_get(>dev, "pwm");
+   count = of_property_count_strings(pdev->dev.of_node, "clock-names");
+   if (count == 2)
+   pc->pclk = devm_clk_get(>dev, "pclk");
+   else
+   pc->pclk = pc->clk;
+
+   if (IS_ERR(pc->clk)) {
+   ret = PTR_ERR(pc->clk);
+   if (ret != -EPROBE_DEFER)
+   dev_err(>dev, "Can't get bus clk: %d\n", ret);
+   return ret;
+   }
+
+   if (IS_ERR(pc->pclk)) {
+   ret = PTR_ERR(pc->pclk);
+   if (ret != -EPROBE_DEFER)
+   dev_err(>dev, "Can't get APB clk: %d\n", ret);
+   return ret;
+   }
 
ret = clk_prepare_enable(pc->clk);
-   if (ret)
+   if (ret) {
+   dev_err(>dev, "Can't prepare enable bus clk: %d\n", ret);
return ret;
+   }
+
+   ret = clk_prepare(pc->pclk);
+   if (ret) {
+   dev_err(>dev, "Can't prepare APB clk: %d\n", ret);
+   goto err_clk;
+   }
 
platform_set_drvdata(pdev, pc);
 
@@ -368,12 +394,20 @@ static int rockchip_pwm_probe(struct platform_device 
*pdev)
if (ret < 0) {
clk_unprepare(pc->clk);
dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
+   goto err_pclk;
}
 
/* Keep the PWM clk enabled if the PWM appears to be up and running. */
if (!pwm_is_enabled(pc->chip.pwms))
clk_disable(pc->clk);
 
+   return 0;
+
+err_pclk:
+   clk_unprepare(pc->pclk);
+err_clk:
+ 

[PATCH v2 3/7] pwm: rockchip: Remove the dumplicate rockchip_pwm_ops ops

[David Wu]

The rockchip_pwm_ops_v1 and rockchip_pwm_ops_v2 ops are the same
struct members, remove one of them.

Signed-off-by: David Wu 
---
 drivers/pwm/pwm-rockchip.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c
index cd45f17..85f9515 100644
--- a/drivers/pwm/pwm-rockchip.c
+++ b/drivers/pwm/pwm-rockchip.c
@@ -255,13 +255,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, 
struct pwm_device *pwm,
return ret;
 }
 
-static const struct pwm_ops rockchip_pwm_ops_v1 = {
-   .get_state = rockchip_pwm_get_state,
-   .apply = rockchip_pwm_apply,
-   .owner = THIS_MODULE,
-};
-
-static const struct pwm_ops rockchip_pwm_ops_v2 = {
+static const struct pwm_ops rockchip_pwm_ops = {
.get_state = rockchip_pwm_get_state,
.apply = rockchip_pwm_apply,
.owner = THIS_MODULE,
@@ -275,7 +269,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
.ctrl = 0x0c,
},
.prescaler = 2,
-   .ops = _pwm_ops_v1,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v1,
.get_state = rockchip_pwm_get_state_v1,
 };
@@ -289,7 +283,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
},
.prescaler = 1,
.supports_polarity = true,
-   .ops = _pwm_ops_v2,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
 };
@@ -303,7 +297,7 @@ static int rockchip_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
},
.prescaler = 1,
.supports_polarity = true,
-   .ops = _pwm_ops_v2,
+   .ops = _pwm_ops,
.set_enable = rockchip_pwm_set_enable_v2,
.get_state = rockchip_pwm_get_state_v2,
 };
-- 
1.9.1

Re: [PATCH 0/2] exec: Use sane stack rlimit for setuid exec

[Kees Cook]

On Fri, Jul 7, 2017 at 1:04 PM, Linus Torvalds
 wrote:
> On Fri, Jul 7, 2017 at 12:56 PM, Kees Cook  wrote:
>> As discussed with Linus and Andy, we need to reset the stack rlimit
>> before we do memory layouts when execing a privilege-gaining (e.g.
>> setuid) program. This moves security_bprm_secureexec() earlier (with
>> required changes), and then lowers the stack limit when appropriate.
>
> Looks sane to me, and that first patch looks like a nice cleanup
> regardless - the old semantics were insane.
>
> But yes, we should have more people look at this, particular have the
> security module people look at that first patch to make sure it is the
> right thing to do for their policies, and make sure that everybody's
> bprm_secureexec() function actually looks at the creds in the brmp,
> not "current" (well, maybe they compare the two, which makes tons of
> sense, and which the old  placement didn't sanely support).
>
> It looks like Kees went through the security modules, but having the
> people involved double-check is a good good idea.

Updated tree here, I'll send the series in email on Monday:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec

This should fix the missed bprm->cred->security and breaks out each
step into logical pieces in case we need to sanely bisect.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH 0/2] exec: Use sane stack rlimit for setuid exec

[Kees Cook]

On Fri, Jul 7, 2017 at 1:04 PM, Linus Torvalds
 wrote:
> On Fri, Jul 7, 2017 at 12:56 PM, Kees Cook  wrote:
>> As discussed with Linus and Andy, we need to reset the stack rlimit
>> before we do memory layouts when execing a privilege-gaining (e.g.
>> setuid) program. This moves security_bprm_secureexec() earlier (with
>> required changes), and then lowers the stack limit when appropriate.
>
> Looks sane to me, and that first patch looks like a nice cleanup
> regardless - the old semantics were insane.
>
> But yes, we should have more people look at this, particular have the
> security module people look at that first patch to make sure it is the
> right thing to do for their policies, and make sure that everybody's
> bprm_secureexec() function actually looks at the creds in the brmp,
> not "current" (well, maybe they compare the two, which makes tons of
> sense, and which the old  placement didn't sanely support).
>
> It looks like Kees went through the security modules, but having the
> people involved double-check is a good good idea.

Updated tree here, I'll send the series in email on Monday:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec

This should fix the missed bprm->cred->security and breaks out each
step into logical pieces in case we need to sanely bisect.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

[Alexander Duyck]

On Fri, Jul 7, 2017 at 7:04 PM, Casey Leedom  wrote:
>   Okay, thanks for the note Alexander.  I'll have to look more closely at
> the patch on Monday and try it out on one of the targeted systems to verify
> the semantics you describe.
>
>   However, that said, there is no way to tell a priori where a device will
> send TLPs.  To simply assume that all TLPs will be directed towards the Root
> Complex is a big assumption.  Only the device and the code controlling it
> know where the TLPs will be directed.  That's why there are changes required
> in the cxgb4 driver.  For instance, the code in
> drivers/net/ethernet/chelsio./cxgb4/sge.c: t4_sge_alloc_rxq() knows that
> it's allocating Free List Buffers in Host Memory and that the RX Queues that
> it's allocating in the Hardware will eventually send Ingress Data to those
> Free List Buffers.  (And similarly for the Free List Buffer Pointer Queue
> with respect to DMA Reads from the host.)  In that routine we explicitly
> configure the Hardware to use/not-use the Relaxed Ordering Attribute via the
> FW_IQ_CMD_FL0FETCHRO and FW_IQ_CMD_FL0DATARO flags.  Basically we're
> conditionally setting them based on the desirability of sending Relaxed
> Ordering TLPs to the Root Complex.  (And we would perform the same kind of
> check for an nVME application ... which brings us to ...)

The general idea with this is to keep this simple. In the vast
majority of cases the assumption is that a device will be sending data
back and forth from system memory. As such the mostly likely thing
that any given device will be interacting with is the root complex. By
making it so that we disable relaxed ordering if the root complex says
we can't support it seems like the simplest and most direct solution
to avoid the issue of us sending any requests with relaxed ordering
enabled TLPs to the root complex.

With that said what we are getting into are subtleties that end up
impacting devices that perform peer-to-peer operations and I don't
suspect that peer-to-peer is really all that common. Ideally my
thought on this is that if there is something in the setup that cannot
support relaxed ordering we should be disabling relaxed ordering and
then re-enabling it in the drivers that have special peer-to-peer
routes that they are aware of. This should help to simplify things for
cases such as a function being direct assigned as the configuration
space should be passed through to the guest with the relaxed ordering
attribute disabled, and that status will be visible to the guest. If
we just use the quirk we lose that and it becomes problematic if a
function is direct assigned on a system that doesn't support relaxed
ordering as the guest has no visibility into the host's quirks.

>   And what would be the code using these patch APIs to set up a Peer-to-Peer
> nVME-style application?  In that case we'd need the Chelsio adapter's PCIe
> Capability Device Control[Relaxed Ordering Enable] set for the nVME
> application ... and we would avoid programming the Chelsio Hardware to use
> Relaxed Ordering for TLPs directed at the Root Complex.  Thus we would be in
> a position where some TLPs being emitted by the device to Peer devices would
> have Relaxed Ordering set and some directed at the Root Complex would not.
> And the only way for that to work is if the source device's Device
> Control[Relaxed Ordering Enable] is set ...

Right. I admit this is pushing extra complexity into the driver, but
what else would you have us do?

The idea here is more of a lock-out tag-out type setup. We go in and
disable relaxed ordering on the devices if it isn't safe to send a
relaxed ordering TLP to the root complex. We then leave it up to the
driver to go through and re-enable it if the driver knows enough about
how it works and what kind of transactions it might issue. I'm not
saying we have to leave the relaxed ordering bit disabled in the
control register. I'm saying we disable it at first, and then leave it
up to the device driver to re-enable it if it needs the functionality
for something that is other than root-complex based and it knows it
can avoid sending the frames to the root complex. Ideally such a
driver would also clean up after itself if removed so that it leaves
the device in the same state it found it in.

Maybe we don't even really need patch 3/3 in this series. If disabling
the relaxed ordering bit in the configuration space already disabled
relaxed ordering for the entire device then this patch doesn't even
really do anything then right? The device takes care of it already for
us so we could probably just drop this patch as it currently stands.

If that is the case maybe we need to refocus patch 3/3 on re-enabling
relaxed ordering for that nVME specific case. That might be beyond the
scope of what Ding can handle though, and I am not familiar with the
Chelsio hardware either. So that might be something that is best left
to you as a follow-up patch.

>   Finally, setting 

Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

[Alexander Duyck]

On Fri, Jul 7, 2017 at 7:04 PM, Casey Leedom  wrote:
>   Okay, thanks for the note Alexander.  I'll have to look more closely at
> the patch on Monday and try it out on one of the targeted systems to verify
> the semantics you describe.
>
>   However, that said, there is no way to tell a priori where a device will
> send TLPs.  To simply assume that all TLPs will be directed towards the Root
> Complex is a big assumption.  Only the device and the code controlling it
> know where the TLPs will be directed.  That's why there are changes required
> in the cxgb4 driver.  For instance, the code in
> drivers/net/ethernet/chelsio./cxgb4/sge.c: t4_sge_alloc_rxq() knows that
> it's allocating Free List Buffers in Host Memory and that the RX Queues that
> it's allocating in the Hardware will eventually send Ingress Data to those
> Free List Buffers.  (And similarly for the Free List Buffer Pointer Queue
> with respect to DMA Reads from the host.)  In that routine we explicitly
> configure the Hardware to use/not-use the Relaxed Ordering Attribute via the
> FW_IQ_CMD_FL0FETCHRO and FW_IQ_CMD_FL0DATARO flags.  Basically we're
> conditionally setting them based on the desirability of sending Relaxed
> Ordering TLPs to the Root Complex.  (And we would perform the same kind of
> check for an nVME application ... which brings us to ...)

The general idea with this is to keep this simple. In the vast
majority of cases the assumption is that a device will be sending data
back and forth from system memory. As such the mostly likely thing
that any given device will be interacting with is the root complex. By
making it so that we disable relaxed ordering if the root complex says
we can't support it seems like the simplest and most direct solution
to avoid the issue of us sending any requests with relaxed ordering
enabled TLPs to the root complex.

With that said what we are getting into are subtleties that end up
impacting devices that perform peer-to-peer operations and I don't
suspect that peer-to-peer is really all that common. Ideally my
thought on this is that if there is something in the setup that cannot
support relaxed ordering we should be disabling relaxed ordering and
then re-enabling it in the drivers that have special peer-to-peer
routes that they are aware of. This should help to simplify things for
cases such as a function being direct assigned as the configuration
space should be passed through to the guest with the relaxed ordering
attribute disabled, and that status will be visible to the guest. If
we just use the quirk we lose that and it becomes problematic if a
function is direct assigned on a system that doesn't support relaxed
ordering as the guest has no visibility into the host's quirks.

>   And what would be the code using these patch APIs to set up a Peer-to-Peer
> nVME-style application?  In that case we'd need the Chelsio adapter's PCIe
> Capability Device Control[Relaxed Ordering Enable] set for the nVME
> application ... and we would avoid programming the Chelsio Hardware to use
> Relaxed Ordering for TLPs directed at the Root Complex.  Thus we would be in
> a position where some TLPs being emitted by the device to Peer devices would
> have Relaxed Ordering set and some directed at the Root Complex would not.
> And the only way for that to work is if the source device's Device
> Control[Relaxed Ordering Enable] is set ...

Right. I admit this is pushing extra complexity into the driver, but
what else would you have us do?

The idea here is more of a lock-out tag-out type setup. We go in and
disable relaxed ordering on the devices if it isn't safe to send a
relaxed ordering TLP to the root complex. We then leave it up to the
driver to go through and re-enable it if the driver knows enough about
how it works and what kind of transactions it might issue. I'm not
saying we have to leave the relaxed ordering bit disabled in the
control register. I'm saying we disable it at first, and then leave it
up to the device driver to re-enable it if it needs the functionality
for something that is other than root-complex based and it knows it
can avoid sending the frames to the root complex. Ideally such a
driver would also clean up after itself if removed so that it leaves
the device in the same state it found it in.

Maybe we don't even really need patch 3/3 in this series. If disabling
the relaxed ordering bit in the configuration space already disabled
relaxed ordering for the entire device then this patch doesn't even
really do anything then right? The device takes care of it already for
us so we could probably just drop this patch as it currently stands.

If that is the case maybe we need to refocus patch 3/3 on re-enabling
relaxed ordering for that nVME specific case. That might be beyond the
scope of what Ding can handle though, and I am not familiar with the
Chelsio hardware either. So that might be something that is best left
to you as a follow-up patch.

>   Finally, setting aside my disagreements 

Re: [PATCH v3] acpi: handle the acpi hotplug schedule error

[jlee]

Hi Rafael,

On Fri, Jul 07, 2017 at 03:16:40PM +0200, Rafael J. Wysocki wrote:
> On Friday, July 07, 2017 02:22:42 PM Lee, Chun-Yi wrote:
> > Kernel should decrements the reference count of acpi device
> > when the scheduling of acpi hotplug work failed, and evaluates
> > _OST to notify BIOS the failure.
> > 
> > v3:
> > More simplify the code. (Rafael J. Wysocki)
> > 
> > v2:
> > To simplify the code. (Andy Shevchenko)
> > 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Len Brown 
> > Cc: Michal Hocko 
> > Reviewed-by: Andy Shevchenko 
> > Signed-off-by: "Lee, Chun-Yi" 
> 
> This has been applied too AFAICS.
> 

I saw patches in linux-next.

Thanks for your review.

Joey Lee

> > ---
> >  drivers/acpi/bus.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 784bda6..9d4fea6 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -432,11 +432,15 @@ static void acpi_bus_notify(acpi_handle handle, u32 
> > type, void *data)
> > (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
> > driver->ops.notify(adev, type);
> >  
> > -   if (hotplug_event && ACPI_SUCCESS(acpi_hotplug_schedule(adev, type)))
> > +   if (!hotplug_event) {
> > +   acpi_bus_put_acpi_device(adev);
> > +   return;
> > +   }
> > +
> > +   if (ACPI_SUCCESS(acpi_hotplug_schedule(adev, type)))
> > return;
> >  
> > acpi_bus_put_acpi_device(adev);
> > -   return;
> >  
> >   err:
> > acpi_evaluate_ost(handle, type, ost_code, NULL);
> > 
> 

Re: [PATCH v3] acpi: handle the acpi hotplug schedule error

[jlee]

Hi Rafael,

On Fri, Jul 07, 2017 at 03:16:40PM +0200, Rafael J. Wysocki wrote:
> On Friday, July 07, 2017 02:22:42 PM Lee, Chun-Yi wrote:
> > Kernel should decrements the reference count of acpi device
> > when the scheduling of acpi hotplug work failed, and evaluates
> > _OST to notify BIOS the failure.
> > 
> > v3:
> > More simplify the code. (Rafael J. Wysocki)
> > 
> > v2:
> > To simplify the code. (Andy Shevchenko)
> > 
> > Cc: "Rafael J. Wysocki" 
> > Cc: Len Brown 
> > Cc: Michal Hocko 
> > Reviewed-by: Andy Shevchenko 
> > Signed-off-by: "Lee, Chun-Yi" 
> 
> This has been applied too AFAICS.
> 

I saw patches in linux-next.

Thanks for your review.

Joey Lee

> > ---
> >  drivers/acpi/bus.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 784bda6..9d4fea6 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -432,11 +432,15 @@ static void acpi_bus_notify(acpi_handle handle, u32 
> > type, void *data)
> > (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
> > driver->ops.notify(adev, type);
> >  
> > -   if (hotplug_event && ACPI_SUCCESS(acpi_hotplug_schedule(adev, type)))
> > +   if (!hotplug_event) {
> > +   acpi_bus_put_acpi_device(adev);
> > +   return;
> > +   }
> > +
> > +   if (ACPI_SUCCESS(acpi_hotplug_schedule(adev, type)))
> > return;
> >  
> > acpi_bus_put_acpi_device(adev);
> > -   return;
> >  
> >   err:
> > acpi_evaluate_ost(handle, type, ost_code, NULL);
> > 
> 

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Sat, Jul 08, 2017 at 01:40:18AM +0200, Adam Borowski wrote:
> On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> > On 7/6/17, 9:32 AM, "Adam Borowski"  wrote:
> > > Got a reproducible crash on amd64:
>
> > Thanks for the bug report Adam! I'm looking into the failure, and haven't
> > been able to reproduce it yet. I've built my kernel from your tree, and
> > I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> > a failure yet.
> 
> > I have a few questions to guide my debugging.
> > 
> > - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> > cores.
> > - Which version of gcc are you using to compile the kernel? I’m using 
> > gcc-6.2.0-5ubuntu12.
> > - Are the failures always in exactly the same place, and does it fail 100%
> >   of the time or just regularly?
> 
> 6 cores -- all on bare metal.  gcc-7.1.0-9.
> Lemme try with gcc-6, a different config or in a VM.

I've tried the following:
* gcc-6, defconfig (+btrfs obviously)
* gcc-7, defconfig
* gcc-6, my regular config
* gcc-7, my regular config
* gcc-7, debug + UBSAN + etc
* gcc-7, defconfig, qemu-kvm with only 1 core

Every build with gcc-7 reproduces the crash, every with gcc-6 does not.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

Re: [PATCH v2 3/4] btrfs: Add zstd support

[Adam Borowski]

On Sat, Jul 08, 2017 at 01:40:18AM +0200, Adam Borowski wrote:
> On Fri, Jul 07, 2017 at 11:17:49PM +, Nick Terrell wrote:
> > On 7/6/17, 9:32 AM, "Adam Borowski"  wrote:
> > > Got a reproducible crash on amd64:
>
> > Thanks for the bug report Adam! I'm looking into the failure, and haven't
> > been able to reproduce it yet. I've built my kernel from your tree, and
> > I ran your script with the kernel.tar tarball 100 times, but haven't gotten
> > a failure yet.
> 
> > I have a few questions to guide my debugging.
> > 
> > - How many cores are you running with? I’ve run the script with 1, 2, and 4 
> > cores.
> > - Which version of gcc are you using to compile the kernel? I’m using 
> > gcc-6.2.0-5ubuntu12.
> > - Are the failures always in exactly the same place, and does it fail 100%
> >   of the time or just regularly?
> 
> 6 cores -- all on bare metal.  gcc-7.1.0-9.
> Lemme try with gcc-6, a different config or in a VM.

I've tried the following:
* gcc-6, defconfig (+btrfs obviously)
* gcc-7, defconfig
* gcc-6, my regular config
* gcc-7, my regular config
* gcc-7, debug + UBSAN + etc
* gcc-7, defconfig, qemu-kvm with only 1 core

Every build with gcc-7 reproduces the crash, every with gcc-6 does not.

-- 
⢀⣴⠾⠻⢶⣦⠀ 
⣾⠁⢠⠒⠀⣿⡁ A dumb species has no way to open a tuna can.
⢿⡄⠘⠷⠚⠋⠀ A smart species invents a can opener.
⠈⠳⣄ A master species delegates.

[PATCH] printk: Modify operators of printed_len

[Pierre Kuo]

In 8b1742c9c207, we remove printk-recursion detection code in
vprintk_emit(), where it is the first place that printed_len calculated.
After removing above detection, it seems we can directly assign the
result of log_output to printed_len.

Signed-off-by: Pierre Kuo 
---
 kernel/printk/printk.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863..a2a8cac 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
size_t text_len = 0;
enum log_flags lflags = 0;
unsigned long flags;
-   int printed_len = 0;
+   int printed_len;
bool in_sched = false;
 
if (level == LOGLEVEL_SCHED) {
@@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
if (dict)
lflags |= LOG_PREFIX|LOG_NEWLINE;
 
-   printed_len += log_output(facility, level, lflags, dict, dictlen, text, 
text_len);
+   printed_len = log_output(facility, level, lflags, dict, dictlen, text, 
text_len);
 
logbuf_unlock_irqrestore(flags);
 
-- 
1.7.9.5

[PATCH] printk: Modify operators of printed_len

[Pierre Kuo]

In 8b1742c9c207, we remove printk-recursion detection code in
vprintk_emit(), where it is the first place that printed_len calculated.
After removing above detection, it seems we can directly assign the
result of log_output to printed_len.

Signed-off-by: Pierre Kuo 
---
 kernel/printk/printk.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863..a2a8cac 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1701,7 +1701,7 @@ asmlinkage int vprintk_emit(int facility, int level,
size_t text_len = 0;
enum log_flags lflags = 0;
unsigned long flags;
-   int printed_len = 0;
+   int printed_len;
bool in_sched = false;
 
if (level == LOGLEVEL_SCHED) {
@@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
if (dict)
lflags |= LOG_PREFIX|LOG_NEWLINE;
 
-   printed_len += log_output(facility, level, lflags, dict, dictlen, text, 
text_len);
+   printed_len = log_output(facility, level, lflags, dict, dictlen, text, 
text_len);
 
logbuf_unlock_irqrestore(flags);
 
-- 
1.7.9.5

Re: [PATCH] printk: Modify operators of printed_len

[pierre kuo]

hi Joe
>> > []
>> > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> >
>> > []
>> > > @@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int 
>> > > level,
>> > >   if (dict)
>> > >   lflags |= LOG_PREFIX|LOG_NEWLINE;
>> > >
>> > > - printed_len += log_output(facility, level, lflags, dict, dictlen, 
>> > > text, text_len);
>> > > + printed_len = log_output(facility, level, lflags, dict, dictlen, 
>> > > text, text_len);
>> >
>> > If this is appropriate, this should also remove the
>> > initialization of printed_len and perhaps rename it too.
>>
>> I cannot quite understand the reason why need to rename.
>> printed_len seems meet the meaning we expect for here.
>
> Verbosity.  To me, len would be adequate.
>
> Anyway, the real point was the declaration of printed_len could
> remove the " = 0" as it's now only set once.
Got it and I will resend the patch again.

Appreciate your kind advice.

Re: [PATCH] printk: Modify operators of printed_len

[pierre kuo]

hi Joe
>> > []
>> > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> >
>> > []
>> > > @@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int 
>> > > level,
>> > >   if (dict)
>> > >   lflags |= LOG_PREFIX|LOG_NEWLINE;
>> > >
>> > > - printed_len += log_output(facility, level, lflags, dict, dictlen, 
>> > > text, text_len);
>> > > + printed_len = log_output(facility, level, lflags, dict, dictlen, 
>> > > text, text_len);
>> >
>> > If this is appropriate, this should also remove the
>> > initialization of printed_len and perhaps rename it too.
>>
>> I cannot quite understand the reason why need to rename.
>> printed_len seems meet the meaning we expect for here.
>
> Verbosity.  To me, len would be adequate.
>
> Anyway, the real point was the declaration of printed_len could
> remove the " = 0" as it's now only set once.
Got it and I will resend the patch again.

Appreciate your kind advice.

Re: [PATCH] exec: Limit arg stack to at most _STK_LIM / 4 * 3

[Kees Cook]

On Fri, Jul 7, 2017 at 3:24 PM, Linus Torvalds
 wrote:
> On Fri, Jul 7, 2017 at 11:57 AM, Kees Cook  wrote:
>> To avoid pathological stack usage or the need to special-case setuid
>> execs, just limit all arg stack usage to at most _STK_LIM / 4 * 3 (6MB).
>
> Ok, this I think I should just apply, but would prefer to avoid
> multi-line complex conditionals around things like this.
>
> So how about the attached slightly edited version instead?
>
> I didn't test it (and I'm not really committing it until I get an ack
> or two), but it seemed all ObviouslyCorrect(tm). FamousLastWords(tm).
>
> Comments?

That works for me, thanks. Testing showed the same sane results:

$ ulimit -s 32768
$ ./args
Detected max args size near: 6291023 bytes
$ ulimit -s 24576
$ ./args
Detected max args size near: 6291022 bytes
$ ulimit -s 20480
$ ./args
Detected max args size near: 5242448 bytes
$ ulimit -s 16384
$ ./args
Detected max args size near: 4193871 bytes
$ ulimit -s 8192
$ ./args
Detected max args size near: 2096719 bytes
$ ulimit -s 4096
$ ./args
Detected max args size near: 1048143 bytes

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] exec: Limit arg stack to at most _STK_LIM / 4 * 3

[Kees Cook]

On Fri, Jul 7, 2017 at 3:24 PM, Linus Torvalds
 wrote:
> On Fri, Jul 7, 2017 at 11:57 AM, Kees Cook  wrote:
>> To avoid pathological stack usage or the need to special-case setuid
>> execs, just limit all arg stack usage to at most _STK_LIM / 4 * 3 (6MB).
>
> Ok, this I think I should just apply, but would prefer to avoid
> multi-line complex conditionals around things like this.
>
> So how about the attached slightly edited version instead?
>
> I didn't test it (and I'm not really committing it until I get an ack
> or two), but it seemed all ObviouslyCorrect(tm). FamousLastWords(tm).
>
> Comments?

That works for me, thanks. Testing showed the same sane results:

$ ulimit -s 32768
$ ./args
Detected max args size near: 6291023 bytes
$ ulimit -s 24576
$ ./args
Detected max args size near: 6291022 bytes
$ ulimit -s 20480
$ ./args
Detected max args size near: 5242448 bytes
$ ulimit -s 16384
$ ./args
Detected max args size near: 4193871 bytes
$ ulimit -s 8192
$ ./args
Detected max args size near: 2096719 bytes
$ ulimit -s 4096
$ ./args
Detected max args size near: 1048143 bytes

-Kees

-- 
Kees Cook
Pixel Security

Re: [GIT PULL] writeback error handling fixes (pile #2)

[Linus Torvalds]

On Thu, Jul 6, 2017 at 5:26 AM, Jeff Layton  wrote:
>
> Sorry for the long description, but I think it's important to give good
> background here:

Thanks, this was what I wanted.

Looks good, and pulled - still going through my basic test-builds
before getting pushed out.

  Linus

Re: [GIT PULL] writeback error handling fixes (pile #2)

[Linus Torvalds]

On Thu, Jul 6, 2017 at 5:26 AM, Jeff Layton  wrote:
>
> Sorry for the long description, but I think it's important to give good
> background here:

Thanks, this was what I wanted.

Looks good, and pulled - still going through my basic test-builds
before getting pushed out.

  Linus

Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

[Jens Axboe]

On 07/07/2017 07:51 PM, Goldwyn Rodrigues wrote:
> 
> 
> On 07/04/2017 05:16 PM, Jens Axboe wrote:
>>
>> Please expedite getting this upstream, asap.
>>
> 
> Jens,
> 
> I have posted an updated patch [1] and it is acked by David. Would you
> pick it up or should it go through the btrfs tree (or some other tree)?
> 
> [1] https://patchwork.kernel.org/patch/9825813/

I'm fine with either, I just want it to go in asap. I'm sending off
a pull Monday. David, up to you.

-- 
Jens Axboe

Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

[Jens Axboe]

On 07/07/2017 07:51 PM, Goldwyn Rodrigues wrote:
> 
> 
> On 07/04/2017 05:16 PM, Jens Axboe wrote:
>>
>> Please expedite getting this upstream, asap.
>>
> 
> Jens,
> 
> I have posted an updated patch [1] and it is acked by David. Would you
> pick it up or should it go through the btrfs tree (or some other tree)?
> 
> [1] https://patchwork.kernel.org/patch/9825813/

I'm fine with either, I just want it to go in asap. I'm sending off
a pull Monday. David, up to you.

-- 
Jens Axboe

Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

[Casey Leedom]

  Okay, thanks for the note Alexander.  I'll have to look more closely at
the patch on Monday and try it out on one of the targeted systems to verify
the semantics you describe.

  However, that said, there is no way to tell a priori where a device will
send TLPs.  To simply assume that all TLPs will be directed towards the Root
Complex is a big assumption.  Only the device and the code controlling it
know where the TLPs will be directed.  That's why there are changes required
in the cxgb4 driver.  For instance, the code in
drivers/net/ethernet/chelsio./cxgb4/sge.c: t4_sge_alloc_rxq() knows that
it's allocating Free List Buffers in Host Memory and that the RX Queues that
it's allocating in the Hardware will eventually send Ingress Data to those
Free List Buffers.  (And similarly for the Free List Buffer Pointer Queue
with respect to DMA Reads from the host.)  In that routine we explicitly
configure the Hardware to use/not-use the Relaxed Ordering Attribute via the
FW_IQ_CMD_FL0FETCHRO and FW_IQ_CMD_FL0DATARO flags.  Basically we're
conditionally setting them based on the desirability of sending Relaxed
Ordering TLPs to the Root Complex.  (And we would perform the same kind of
check for an nVME application ... which brings us to ...)

  And what would be the code using these patch APIs to set up a Peer-to-Peer
nVME-style application?  In that case we'd need the Chelsio adapter's PCIe
Capability Device Control[Relaxed Ordering Enable] set for the nVME
application ... and we would avoid programming the Chelsio Hardware to use
Relaxed Ordering for TLPs directed at the Root Complex.  Thus we would be in
a position where some TLPs being emitted by the device to Peer devices would
have Relaxed Ordering set and some directed at the Root Complex would not.
And the only way for that to work is if the source device's Device
Control[Relaxed Ordering Enable] is set ...

  Finally, setting aside my disagreements with the patch, we still have the
code in the cxgb4 driver which explicitly turns on its own Device
Control[Relaxed Ordering Enable] in cxgb4_main.c:
enable_pcie_relaxed_ordering().  So the patch is something of a loop if all
we're doing is testing our own Relaxed Ordering Enable state ...
 
Casey

Re: [PATCH v6 0/3] Add new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag

[Casey Leedom]

  Okay, thanks for the note Alexander.  I'll have to look more closely at
the patch on Monday and try it out on one of the targeted systems to verify
the semantics you describe.

  However, that said, there is no way to tell a priori where a device will
send TLPs.  To simply assume that all TLPs will be directed towards the Root
Complex is a big assumption.  Only the device and the code controlling it
know where the TLPs will be directed.  That's why there are changes required
in the cxgb4 driver.  For instance, the code in
drivers/net/ethernet/chelsio./cxgb4/sge.c: t4_sge_alloc_rxq() knows that
it's allocating Free List Buffers in Host Memory and that the RX Queues that
it's allocating in the Hardware will eventually send Ingress Data to those
Free List Buffers.  (And similarly for the Free List Buffer Pointer Queue
with respect to DMA Reads from the host.)  In that routine we explicitly
configure the Hardware to use/not-use the Relaxed Ordering Attribute via the
FW_IQ_CMD_FL0FETCHRO and FW_IQ_CMD_FL0DATARO flags.  Basically we're
conditionally setting them based on the desirability of sending Relaxed
Ordering TLPs to the Root Complex.  (And we would perform the same kind of
check for an nVME application ... which brings us to ...)

  And what would be the code using these patch APIs to set up a Peer-to-Peer
nVME-style application?  In that case we'd need the Chelsio adapter's PCIe
Capability Device Control[Relaxed Ordering Enable] set for the nVME
application ... and we would avoid programming the Chelsio Hardware to use
Relaxed Ordering for TLPs directed at the Root Complex.  Thus we would be in
a position where some TLPs being emitted by the device to Peer devices would
have Relaxed Ordering set and some directed at the Root Complex would not.
And the only way for that to work is if the source device's Device
Control[Relaxed Ordering Enable] is set ...

  Finally, setting aside my disagreements with the patch, we still have the
code in the cxgb4 driver which explicitly turns on its own Device
Control[Relaxed Ordering Enable] in cxgb4_main.c:
enable_pcie_relaxed_ordering().  So the patch is something of a loop if all
we're doing is testing our own Relaxed Ordering Enable state ...
 
Casey

Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

[Goldwyn Rodrigues]

On 07/04/2017 05:16 PM, Jens Axboe wrote:
> 
> Please expedite getting this upstream, asap.
> 

Jens,

I have posted an updated patch [1] and it is acked by David. Would you
pick it up or should it go through the btrfs tree (or some other tree)?

[1] https://patchwork.kernel.org/patch/9825813/

-- 
Goldwyn

Re: Commit edf064e7c (btrfs: nowait aio support) breaks shells

[Goldwyn Rodrigues]

On 07/04/2017 05:16 PM, Jens Axboe wrote:
> 
> Please expedite getting this upstream, asap.
> 

Jens,

I have posted an updated patch [1] and it is acked by David. Would you
pick it up or should it go through the btrfs tree (or some other tree)?

[1] https://patchwork.kernel.org/patch/9825813/

-- 
Goldwyn

Re: [PATCH] watchdog: stm32_iwdg: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:28 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/stm32_iwdg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 6c501b7..be64a86 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -140,7 +140,7 @@ static const struct watchdog_info stm32_iwdg_info = {
.identity   = "STM32 Independent Watchdog",
  };
  
-static struct watchdog_ops stm32_iwdg_ops = {

+static const struct watchdog_ops stm32_iwdg_ops = {
.owner  = THIS_MODULE,
.start  = stm32_iwdg_start,
.ping   = stm32_iwdg_ping,

Re: [PATCH] watchdog: stm32_iwdg: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:28 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/stm32_iwdg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 6c501b7..be64a86 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -140,7 +140,7 @@ static const struct watchdog_info stm32_iwdg_info = {
.identity   = "STM32 Independent Watchdog",
  };
  
-static struct watchdog_ops stm32_iwdg_ops = {

+static const struct watchdog_ops stm32_iwdg_ops = {
.owner  = THIS_MODULE,
.start  = stm32_iwdg_start,
.ping   = stm32_iwdg_ping,

Re: [PATCH] watchdog: coh901327_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:33 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/coh901327_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
index 38dd60f0..4410337 100644
--- a/drivers/watchdog/coh901327_wdt.c
+++ b/drivers/watchdog/coh901327_wdt.c
@@ -218,7 +218,7 @@ static const struct watchdog_info coh901327_ident = {
.identity = DRV_NAME,
  };
  
-static struct watchdog_ops coh901327_ops = {

+static const struct watchdog_ops coh901327_ops = {
.owner = THIS_MODULE,
.start = coh901327_start,
.stop = coh901327_stop,

Re: [PATCH] watchdog: coh901327_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:33 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/coh901327_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/coh901327_wdt.c b/drivers/watchdog/coh901327_wdt.c
index 38dd60f0..4410337 100644
--- a/drivers/watchdog/coh901327_wdt.c
+++ b/drivers/watchdog/coh901327_wdt.c
@@ -218,7 +218,7 @@ static const struct watchdog_info coh901327_ident = {
.identity = DRV_NAME,
  };
  
-static struct watchdog_ops coh901327_ops = {

+static const struct watchdog_ops coh901327_ops = {
.owner = THIS_MODULE,
.start = coh901327_start,
.stop = coh901327_stop,

Re: [PATCH] watchdog: stm32_iwdg: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:28 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/stm32_iwdg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 6c501b7..be64a86 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -140,7 +140,7 @@ static const struct watchdog_info stm32_iwdg_info = {
.identity   = "STM32 Independent Watchdog",
  };
  
-static struct watchdog_ops stm32_iwdg_ops = {

+static const struct watchdog_ops stm32_iwdg_ops = {
.owner  = THIS_MODULE,
.start  = stm32_iwdg_start,
.ping   = stm32_iwdg_ping,

Re: [PATCH] watchdog: stm32_iwdg: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:28 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/stm32_iwdg.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index 6c501b7..be64a86 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -140,7 +140,7 @@ static const struct watchdog_info stm32_iwdg_info = {
.identity   = "STM32 Independent Watchdog",
  };
  
-static struct watchdog_ops stm32_iwdg_ops = {

+static const struct watchdog_ops stm32_iwdg_ops = {
.owner  = THIS_MODULE,
.start  = stm32_iwdg_start,
.ping   = stm32_iwdg_ping,

Re: [PATCH] watchdog: it87_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:23 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/it87_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
index dd1e7ea..e96faea 100644
--- a/drivers/watchdog/it87_wdt.c
+++ b/drivers/watchdog/it87_wdt.c
@@ -253,7 +253,7 @@ static const struct watchdog_info ident = {
.identity = WATCHDOG_NAME,
  };
  
-static struct watchdog_ops wdt_ops = {

+static const struct watchdog_ops wdt_ops = {
.owner = THIS_MODULE,
.start = wdt_start,
.stop = wdt_stop,

Re: [PATCH] watchdog: it87_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:23 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/it87_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
index dd1e7ea..e96faea 100644
--- a/drivers/watchdog/it87_wdt.c
+++ b/drivers/watchdog/it87_wdt.c
@@ -253,7 +253,7 @@ static const struct watchdog_info ident = {
.identity = WATCHDOG_NAME,
  };
  
-static struct watchdog_ops wdt_ops = {

+static const struct watchdog_ops wdt_ops = {
.owner = THIS_MODULE,
.start = wdt_start,
.stop = wdt_stop,

Re: [PATCH] watchdog: ts72xx_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:18 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/ts72xx_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/ts72xx_wdt.c b/drivers/watchdog/ts72xx_wdt.c
index 17c25da..811e43c 100644
--- a/drivers/watchdog/ts72xx_wdt.c
+++ b/drivers/watchdog/ts72xx_wdt.c
@@ -112,7 +112,7 @@ static const struct watchdog_info ts72xx_wdt_ident = {
.identity   = "TS-72XX WDT",
  };
  
-static struct watchdog_ops ts72xx_wdt_ops = {

+static const struct watchdog_ops ts72xx_wdt_ops = {
.owner  = THIS_MODULE,
.start  = ts72xx_wdt_start,
.stop   = ts72xx_wdt_stop,

Re: [PATCH] watchdog: ts72xx_wdt: constify watchdog_ops structure

[Guenter Roeck]

On 07/07/2017 05:18 PM, Gustavo A. R. Silva wrote:

Check for watchdog_ops structures that are only stored in the ops field of
a watchdog_device structure. This field is declared const, so watchdog_ops
structures that have this property can be declared as const also.

This issue was detected using Coccinelle and the following semantic patch:

@r
disable optional_qualifier@
identifier i;
position p;
@@
static struct watchdog_ops i@p = { ... };

@ok@
identifier r.i;
struct watchdog_device e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct watchdog_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
  struct watchdog_ops i = { ... };

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Guenter Roeck 


---
  drivers/watchdog/ts72xx_wdt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/ts72xx_wdt.c b/drivers/watchdog/ts72xx_wdt.c
index 17c25da..811e43c 100644
--- a/drivers/watchdog/ts72xx_wdt.c
+++ b/drivers/watchdog/ts72xx_wdt.c
@@ -112,7 +112,7 @@ static const struct watchdog_info ts72xx_wdt_ident = {
.identity   = "TS-72XX WDT",
  };
  
-static struct watchdog_ops ts72xx_wdt_ops = {

+static const struct watchdog_ops ts72xx_wdt_ops = {
.owner  = THIS_MODULE,
.start  = ts72xx_wdt_start,
.stop   = ts72xx_wdt_stop,

[PATCH V3 0/3] Refine numa_emulation

[Wei Yang]

My previous patch "x86/mm/numa: Remove numa_nodemask_from_meminfo()" hits a
problem in numa_emulation. The reason is numa_nodes_parsed is not set
correctly after emulation.

This patch set tries to fix this and also with two code refine.

Detailed discussions are in this thread:

https://lkml.org/lkml/2017/3/13/1230

and test result is posted :

https://lkml.org/lkml/2017/4/10/641

V3:
* remote the error branch and split the loop into a function
* refine the comment

V2:
* refresh the change log based on David comments
* use nodes_clear()

Wei Yang (3):
  x86/numa_emulation: refine the calculation of max_emu_nid and
dfl_phys_nid
  x86/numa_emulation: assign physnode_mask directly from
numa_nodes_parsed
  x86/numa_emulation: restructures numa_nodes_parsed from emulated nodes

 arch/x86/mm/numa_emulation.c | 55 ++--
 1 file changed, 32 insertions(+), 23 deletions(-)

-- 
2.11.0

[PATCH V3 0/3] Refine numa_emulation

[Wei Yang]

My previous patch "x86/mm/numa: Remove numa_nodemask_from_meminfo()" hits a
problem in numa_emulation. The reason is numa_nodes_parsed is not set
correctly after emulation.

This patch set tries to fix this and also with two code refine.

Detailed discussions are in this thread:

https://lkml.org/lkml/2017/3/13/1230

and test result is posted :

https://lkml.org/lkml/2017/4/10/641

V3:
* remote the error branch and split the loop into a function
* refine the comment

V2:
* refresh the change log based on David comments
* use nodes_clear()

Wei Yang (3):
  x86/numa_emulation: refine the calculation of max_emu_nid and
dfl_phys_nid
  x86/numa_emulation: assign physnode_mask directly from
numa_nodes_parsed
  x86/numa_emulation: restructures numa_nodes_parsed from emulated nodes

 arch/x86/mm/numa_emulation.c | 55 ++--
 1 file changed, 32 insertions(+), 23 deletions(-)

-- 
2.11.0

[PATCH V3 1/3] x86/numa_emulation: refine the calculation of max_emu_nid and dfl_phys_nid

[Wei Yang]

max_emu_nid and dfl_phys_nid is calculated from emu_nid_to_phys[], which is
calculated in split_nodes_xxx_interleave(). From the logic in these
functions, it is assured the emu_nid_to_phys[] has meaningful value if it
return successfully and ensures dfl_phys_nid will get a valid value.

This patch removes the error branch to check invalid dfl_phys_nid and
abstract this part to a function for readability.

Signed-off-by: Wei Yang 
---
 arch/x86/mm/numa_emulation.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index a8f90ce3dedf..a6d55308660f 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -280,6 +280,22 @@ static int __init split_nodes_size_interleave(struct 
numa_meminfo *ei,
return 0;
 }
 
+int __init setup_emu2phys_nid(int *dfl_phys_nid)
+{
+   int i, max_emu_nid = 0;
+
+   *dfl_phys_nid = NUMA_NO_NODE;
+   for (i = 0; i < ARRAY_SIZE(emu_nid_to_phys); i++) {
+   if (emu_nid_to_phys[i] != NUMA_NO_NODE) {
+   max_emu_nid = i;
+   if (*dfl_phys_nid == NUMA_NO_NODE)
+   *dfl_phys_nid = emu_nid_to_phys[i];
+   }
+   }
+
+   return max_emu_nid;
+}
+
 /**
  * numa_emulation - Emulate NUMA nodes
  * @numa_meminfo: NUMA configuration to massage
@@ -376,19 +392,7 @@ void __init numa_emulation(struct numa_meminfo 
*numa_meminfo, int numa_dist_cnt)
 * Determine the max emulated nid and the default phys nid to use
 * for unmapped nodes.
 */
-   max_emu_nid = 0;
-   dfl_phys_nid = NUMA_NO_NODE;
-   for (i = 0; i < ARRAY_SIZE(emu_nid_to_phys); i++) {
-   if (emu_nid_to_phys[i] != NUMA_NO_NODE) {
-   max_emu_nid = i;
-   if (dfl_phys_nid == NUMA_NO_NODE)
-   dfl_phys_nid = emu_nid_to_phys[i];
-   }
-   }
-   if (dfl_phys_nid == NUMA_NO_NODE) {
-   pr_warning("NUMA: Warning: can't determine default physical 
node, disabling emulation\n");
-   goto no_emu;
-   }
+   max_emu_nid = setup_emu2phys_nid(_phys_nid);
 
/* commit */
*numa_meminfo = ei;
-- 
2.11.0

[PATCH V3 1/3] x86/numa_emulation: refine the calculation of max_emu_nid and dfl_phys_nid

[Wei Yang]

max_emu_nid and dfl_phys_nid is calculated from emu_nid_to_phys[], which is
calculated in split_nodes_xxx_interleave(). From the logic in these
functions, it is assured the emu_nid_to_phys[] has meaningful value if it
return successfully and ensures dfl_phys_nid will get a valid value.

This patch removes the error branch to check invalid dfl_phys_nid and
abstract this part to a function for readability.

Signed-off-by: Wei Yang 
---
 arch/x86/mm/numa_emulation.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index a8f90ce3dedf..a6d55308660f 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -280,6 +280,22 @@ static int __init split_nodes_size_interleave(struct 
numa_meminfo *ei,
return 0;
 }
 
+int __init setup_emu2phys_nid(int *dfl_phys_nid)
+{
+   int i, max_emu_nid = 0;
+
+   *dfl_phys_nid = NUMA_NO_NODE;
+   for (i = 0; i < ARRAY_SIZE(emu_nid_to_phys); i++) {
+   if (emu_nid_to_phys[i] != NUMA_NO_NODE) {
+   max_emu_nid = i;
+   if (*dfl_phys_nid == NUMA_NO_NODE)
+   *dfl_phys_nid = emu_nid_to_phys[i];
+   }
+   }
+
+   return max_emu_nid;
+}
+
 /**
  * numa_emulation - Emulate NUMA nodes
  * @numa_meminfo: NUMA configuration to massage
@@ -376,19 +392,7 @@ void __init numa_emulation(struct numa_meminfo 
*numa_meminfo, int numa_dist_cnt)
 * Determine the max emulated nid and the default phys nid to use
 * for unmapped nodes.
 */
-   max_emu_nid = 0;
-   dfl_phys_nid = NUMA_NO_NODE;
-   for (i = 0; i < ARRAY_SIZE(emu_nid_to_phys); i++) {
-   if (emu_nid_to_phys[i] != NUMA_NO_NODE) {
-   max_emu_nid = i;
-   if (dfl_phys_nid == NUMA_NO_NODE)
-   dfl_phys_nid = emu_nid_to_phys[i];
-   }
-   }
-   if (dfl_phys_nid == NUMA_NO_NODE) {
-   pr_warning("NUMA: Warning: can't determine default physical 
node, disabling emulation\n");
-   goto no_emu;
-   }
+   max_emu_nid = setup_emu2phys_nid(_phys_nid);
 
/* commit */
*numa_meminfo = ei;
-- 
2.11.0

[PATCH V3 2/3] x86/numa_emulation: assign physnode_mask directly from numa_nodes_parsed

[Wei Yang]

numa_init() has already called init_func(), which is responsible for
setting numa_nodes_parsed, so use this nodemask instead of re-finding it
when calling numa_emulation().

This patch gets the physnode_mask directly from numa_nodes_parsed. At
the same time, it corrects the comment of these two functions.

Signed-off-by: Wei Yang 
Acked-by: David Rientjes 
Reviewed-by: Borislav Petkov 
---
 arch/x86/mm/numa_emulation.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index a6d55308660f..80904ede2e7f 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -75,13 +75,15 @@ static int __init emu_setup_memblk(struct numa_meminfo *ei,
 
 /*
  * Sets up nr_nodes fake nodes interleaved over physical nodes ranging from 
addr
- * to max_addr.  The return value is the number of nodes allocated.
+ * to max_addr.
+ *
+ * Returns zero on success or negative on error.
  */
 static int __init split_nodes_interleave(struct numa_meminfo *ei,
 struct numa_meminfo *pi,
 u64 addr, u64 max_addr, int nr_nodes)
 {
-   nodemask_t physnode_mask = NODE_MASK_NONE;
+   nodemask_t physnode_mask = numa_nodes_parsed;
u64 size;
int big;
int nid = 0;
@@ -116,9 +118,6 @@ static int __init split_nodes_interleave(struct 
numa_meminfo *ei,
return -1;
}
 
-   for (i = 0; i < pi->nr_blks; i++)
-   node_set(pi->blk[i].nid, physnode_mask);
-
/*
 * Continue to fill physical nodes with fake nodes until there is no
 * memory left on any of them.
@@ -200,13 +199,15 @@ static u64 __init find_end_of_node(u64 start, u64 
max_addr, u64 size)
 
 /*
  * Sets up fake nodes of `size' interleaved over physical nodes ranging from
- * `addr' to `max_addr'.  The return value is the number of nodes allocated.
+ * `addr' to `max_addr'.
+ *
+ * Returns zero on success or negative on error.
  */
 static int __init split_nodes_size_interleave(struct numa_meminfo *ei,
  struct numa_meminfo *pi,
  u64 addr, u64 max_addr, u64 size)
 {
-   nodemask_t physnode_mask = NODE_MASK_NONE;
+   nodemask_t physnode_mask = numa_nodes_parsed;
u64 min_size;
int nid = 0;
int i, ret;
@@ -231,9 +232,6 @@ static int __init split_nodes_size_interleave(struct 
numa_meminfo *ei,
}
size &= FAKE_NODE_MIN_HASH_MASK;
 
-   for (i = 0; i < pi->nr_blks; i++)
-   node_set(pi->blk[i].nid, physnode_mask);
-
/*
 * Fill physical nodes with fake nodes of size until there is no memory
 * left on any of them.
-- 
2.11.0

[PATCH V3 3/3] x86/numa_emulation: restructures numa_nodes_parsed from emulated nodes

[Wei Yang]

By emulating numa, it may contains more or less nodes than physical nodes.
In numa_emulation(), numa_meminfo/numa_distance/__apicid_to_node is
restructured according to emulated nodes, except numa_nodes_parsed.

This patch restructures numa_nodes_parsed from emulated nodes.

Signed-off-by: Wei Yang 
Acked-by: David Rientjes 
---
 arch/x86/mm/numa_emulation.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index 80904ede2e7f..d805162e6045 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -395,6 +395,13 @@ void __init numa_emulation(struct numa_meminfo 
*numa_meminfo, int numa_dist_cnt)
/* commit */
*numa_meminfo = ei;
 
+   /* Make sure numa_nodes_parsed only contains emulated nodes */
+   nodes_clear(numa_nodes_parsed);
+   for (i = 0; i < ARRAY_SIZE(ei.blk); i++)
+   if (ei.blk[i].start != ei.blk[i].end &&
+   ei.blk[i].nid != NUMA_NO_NODE)
+   node_set(ei.blk[i].nid, numa_nodes_parsed);
+
/*
 * Transform __apicid_to_node table to use emulated nids by
 * reverse-mapping phys_nid.  The maps should always exist but fall
-- 
2.11.0

[PATCH V3 2/3] x86/numa_emulation: assign physnode_mask directly from numa_nodes_parsed

[Wei Yang]

numa_init() has already called init_func(), which is responsible for
setting numa_nodes_parsed, so use this nodemask instead of re-finding it
when calling numa_emulation().

This patch gets the physnode_mask directly from numa_nodes_parsed. At
the same time, it corrects the comment of these two functions.

Signed-off-by: Wei Yang 
Acked-by: David Rientjes 
Reviewed-by: Borislav Petkov 
---
 arch/x86/mm/numa_emulation.c | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index a6d55308660f..80904ede2e7f 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -75,13 +75,15 @@ static int __init emu_setup_memblk(struct numa_meminfo *ei,
 
 /*
  * Sets up nr_nodes fake nodes interleaved over physical nodes ranging from 
addr
- * to max_addr.  The return value is the number of nodes allocated.
+ * to max_addr.
+ *
+ * Returns zero on success or negative on error.
  */
 static int __init split_nodes_interleave(struct numa_meminfo *ei,
 struct numa_meminfo *pi,
 u64 addr, u64 max_addr, int nr_nodes)
 {
-   nodemask_t physnode_mask = NODE_MASK_NONE;
+   nodemask_t physnode_mask = numa_nodes_parsed;
u64 size;
int big;
int nid = 0;
@@ -116,9 +118,6 @@ static int __init split_nodes_interleave(struct 
numa_meminfo *ei,
return -1;
}
 
-   for (i = 0; i < pi->nr_blks; i++)
-   node_set(pi->blk[i].nid, physnode_mask);
-
/*
 * Continue to fill physical nodes with fake nodes until there is no
 * memory left on any of them.
@@ -200,13 +199,15 @@ static u64 __init find_end_of_node(u64 start, u64 
max_addr, u64 size)
 
 /*
  * Sets up fake nodes of `size' interleaved over physical nodes ranging from
- * `addr' to `max_addr'.  The return value is the number of nodes allocated.
+ * `addr' to `max_addr'.
+ *
+ * Returns zero on success or negative on error.
  */
 static int __init split_nodes_size_interleave(struct numa_meminfo *ei,
  struct numa_meminfo *pi,
  u64 addr, u64 max_addr, u64 size)
 {
-   nodemask_t physnode_mask = NODE_MASK_NONE;
+   nodemask_t physnode_mask = numa_nodes_parsed;
u64 min_size;
int nid = 0;
int i, ret;
@@ -231,9 +232,6 @@ static int __init split_nodes_size_interleave(struct 
numa_meminfo *ei,
}
size &= FAKE_NODE_MIN_HASH_MASK;
 
-   for (i = 0; i < pi->nr_blks; i++)
-   node_set(pi->blk[i].nid, physnode_mask);
-
/*
 * Fill physical nodes with fake nodes of size until there is no memory
 * left on any of them.
-- 
2.11.0

[PATCH V3 3/3] x86/numa_emulation: restructures numa_nodes_parsed from emulated nodes

[Wei Yang]

By emulating numa, it may contains more or less nodes than physical nodes.
In numa_emulation(), numa_meminfo/numa_distance/__apicid_to_node is
restructured according to emulated nodes, except numa_nodes_parsed.

This patch restructures numa_nodes_parsed from emulated nodes.

Signed-off-by: Wei Yang 
Acked-by: David Rientjes 
---
 arch/x86/mm/numa_emulation.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/mm/numa_emulation.c b/arch/x86/mm/numa_emulation.c
index 80904ede2e7f..d805162e6045 100644
--- a/arch/x86/mm/numa_emulation.c
+++ b/arch/x86/mm/numa_emulation.c
@@ -395,6 +395,13 @@ void __init numa_emulation(struct numa_meminfo 
*numa_meminfo, int numa_dist_cnt)
/* commit */
*numa_meminfo = ei;
 
+   /* Make sure numa_nodes_parsed only contains emulated nodes */
+   nodes_clear(numa_nodes_parsed);
+   for (i = 0; i < ARRAY_SIZE(ei.blk); i++)
+   if (ei.blk[i].start != ei.blk[i].end &&
+   ei.blk[i].nid != NUMA_NO_NODE)
+   node_set(ei.blk[i].nid, numa_nodes_parsed);
+
/*
 * Transform __apicid_to_node table to use emulated nids by
 * reverse-mapping phys_nid.  The maps should always exist but fall
-- 
2.11.0

[PATCH] video: cobalt_lcdfb: constify fb_fix_screeninfo structure

[Gustavo A. R. Silva]

This structure is only used to copy into other structures,
so declare it as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/cobalt_lcdfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/cobalt_lcdfb.c 
b/drivers/video/fbdev/cobalt_lcdfb.c
index 9da90bd..0ef633e 100644
--- a/drivers/video/fbdev/cobalt_lcdfb.c
+++ b/drivers/video/fbdev/cobalt_lcdfb.c
@@ -126,7 +126,7 @@ static void lcd_clear(struct fb_info *info)
lcd_write_control(info, LCD_RESET);
 }
 
-static struct fb_fix_screeninfo cobalt_lcdfb_fix = {
+static const struct fb_fix_screeninfo cobalt_lcdfb_fix = {
.id = "cobalt-lcd",
.type   = FB_TYPE_TEXT,
.type_aux   = FB_AUX_TEXT_MDA,
-- 
2.5.0

[PATCH] video: cobalt_lcdfb: constify fb_fix_screeninfo structure

[Gustavo A. R. Silva]

This structure is only used to copy into other structures,
so declare it as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/cobalt_lcdfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/cobalt_lcdfb.c 
b/drivers/video/fbdev/cobalt_lcdfb.c
index 9da90bd..0ef633e 100644
--- a/drivers/video/fbdev/cobalt_lcdfb.c
+++ b/drivers/video/fbdev/cobalt_lcdfb.c
@@ -126,7 +126,7 @@ static void lcd_clear(struct fb_info *info)
lcd_write_control(info, LCD_RESET);
 }
 
-static struct fb_fix_screeninfo cobalt_lcdfb_fix = {
+static const struct fb_fix_screeninfo cobalt_lcdfb_fix = {
.id = "cobalt-lcd",
.type   = FB_TYPE_TEXT,
.type_aux   = FB_AUX_TEXT_MDA,
-- 
2.5.0

[PATCH] video: xilinxfb: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/xilinxfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index 17dc119..0bda18e 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -110,14 +110,14 @@ static struct xilinxfb_platform_data 
xilinx_fb_default_pdata = {
 /*
  * Here are the default fb_fix_screeninfo and fb_var_screeninfo structures
  */
-static struct fb_fix_screeninfo xilinx_fb_fix = {
+static const struct fb_fix_screeninfo xilinx_fb_fix = {
.id =   "Xilinx",
.type = FB_TYPE_PACKED_PIXELS,
.visual =   FB_VISUAL_TRUECOLOR,
.accel =FB_ACCEL_NONE
 };
 
-static struct fb_var_screeninfo xilinx_fb_var = {
+static const struct fb_var_screeninfo xilinx_fb_var = {
.bits_per_pixel =   BITS_PER_PIXEL,
 
.red =  { RED_SHIFT, 8, 0 },
-- 
2.5.0

[PATCH] video: xilinxfb: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/xilinxfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/xilinxfb.c b/drivers/video/fbdev/xilinxfb.c
index 17dc119..0bda18e 100644
--- a/drivers/video/fbdev/xilinxfb.c
+++ b/drivers/video/fbdev/xilinxfb.c
@@ -110,14 +110,14 @@ static struct xilinxfb_platform_data 
xilinx_fb_default_pdata = {
 /*
  * Here are the default fb_fix_screeninfo and fb_var_screeninfo structures
  */
-static struct fb_fix_screeninfo xilinx_fb_fix = {
+static const struct fb_fix_screeninfo xilinx_fb_fix = {
.id =   "Xilinx",
.type = FB_TYPE_PACKED_PIXELS,
.visual =   FB_VISUAL_TRUECOLOR,
.accel =FB_ACCEL_NONE
 };
 
-static struct fb_var_screeninfo xilinx_fb_var = {
+static const struct fb_var_screeninfo xilinx_fb_var = {
.bits_per_pixel =   BITS_PER_PIXEL,
 
.red =  { RED_SHIFT, 8, 0 },
-- 
2.5.0

[PATCH] video/chips: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/chipsfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 59abdc6..f103665 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -292,7 +292,7 @@ static void chips_hw_init(void)
write_fr(chips_init_fr[i].addr, chips_init_fr[i].data);
 }
 
-static struct fb_fix_screeninfo chipsfb_fix = {
+static const struct fb_fix_screeninfo chipsfb_fix = {
.id =   "C 65550",
.type = FB_TYPE_PACKED_PIXELS,
.visual =   FB_VISUAL_PSEUDOCOLOR,
@@ -309,7 +309,7 @@ static struct fb_fix_screeninfo chipsfb_fix = {
.smem_len = 0x10,   /* 1MB */
 };
 
-static struct fb_var_screeninfo chipsfb_var = {
+static const struct fb_var_screeninfo chipsfb_var = {
.xres = 800,
.yres = 600,
.xres_virtual = 800,
-- 
2.5.0

[PATCH] video/chips: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/chipsfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 59abdc6..f103665 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -292,7 +292,7 @@ static void chips_hw_init(void)
write_fr(chips_init_fr[i].addr, chips_init_fr[i].data);
 }
 
-static struct fb_fix_screeninfo chipsfb_fix = {
+static const struct fb_fix_screeninfo chipsfb_fix = {
.id =   "C 65550",
.type = FB_TYPE_PACKED_PIXELS,
.visual =   FB_VISUAL_PSEUDOCOLOR,
@@ -309,7 +309,7 @@ static struct fb_fix_screeninfo chipsfb_fix = {
.smem_len = 0x10,   /* 1MB */
 };
 
-static struct fb_var_screeninfo chipsfb_var = {
+static const struct fb_var_screeninfo chipsfb_var = {
.xres = 800,
.yres = 600,
.xres_virtual = 800,
-- 
2.5.0

Re: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

[Brian Norris]

On Tue, Jul 04, 2017 at 03:22:52AM +, Chris Packham wrote:
> I had someone mention to me in passing that mtdinfo was failing for them 
> (crashing with some floating point error). I'm wondering if we've 
> created a divide-by-zero problem by reporting 0 erase size in /proc/mtd. 
> I don't have any other info and right now I don't have access to the 
> system I had with the mchp23lcv1024 sram.

Wouldn't be that surprising. mtdinfo is actually developed under the
"ubi-utils" directory of mtd-utils, and UBI wouldn't know what to do
with and SRAM like that.

Brian

Re: [PATCH v5 3/5] mtd: handle partitioning on devices with 0 erasesize

[Brian Norris]

On Tue, Jul 04, 2017 at 03:22:52AM +, Chris Packham wrote:
> I had someone mention to me in passing that mtdinfo was failing for them 
> (crashing with some floating point error). I'm wondering if we've 
> created a divide-by-zero problem by reporting 0 erase size in /proc/mtd. 
> I don't have any other info and right now I don't have access to the 
> system I had with the mchp23lcv1024 sram.

Wouldn't be that surprising. mtdinfo is actually developed under the
"ubi-utils" directory of mtd-utils, and UBI wouldn't know what to do
with and SRAM like that.

Brian

[PATCH] video/mbx: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/mbx/mbxfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/mbx/mbxfb.c b/drivers/video/fbdev/mbx/mbxfb.c
index 698df954..539b85d 100644
--- a/drivers/video/fbdev/mbx/mbxfb.c
+++ b/drivers/video/fbdev/mbx/mbxfb.c
@@ -79,7 +79,7 @@ struct mbxfb_info {
 
 };
 
-static struct fb_var_screeninfo mbxfb_default = {
+static const struct fb_var_screeninfo mbxfb_default = {
.xres = 640,
.yres = 480,
.xres_virtual = 640,
@@ -102,7 +102,7 @@ static struct fb_var_screeninfo mbxfb_default = {
.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 };
 
-static struct fb_fix_screeninfo mbxfb_fix = {
+static const struct fb_fix_screeninfo mbxfb_fix = {
.id = "MBX",
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
-- 
2.5.0

[PATCH] video/mbx: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/video/fbdev/mbx/mbxfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/mbx/mbxfb.c b/drivers/video/fbdev/mbx/mbxfb.c
index 698df954..539b85d 100644
--- a/drivers/video/fbdev/mbx/mbxfb.c
+++ b/drivers/video/fbdev/mbx/mbxfb.c
@@ -79,7 +79,7 @@ struct mbxfb_info {
 
 };
 
-static struct fb_var_screeninfo mbxfb_default = {
+static const struct fb_var_screeninfo mbxfb_default = {
.xres = 640,
.yres = 480,
.xres_virtual = 640,
@@ -102,7 +102,7 @@ static struct fb_var_screeninfo mbxfb_default = {
.sync = FB_SYNC_HOR_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
 };
 
-static struct fb_fix_screeninfo mbxfb_fix = {
+static const struct fb_fix_screeninfo mbxfb_fix = {
.id = "MBX",
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_TRUECOLOR,
-- 
2.5.0

[GIT PULL] MD update for 4.13

[Shaohua Li]

Hi,
Please pull MD update for 4.13. This update only includes several bug fixes:

- Neil Brown fixes a deadlock in MD suspend and a potential bug in bio 
allocation.
- Mikulas Patocka fixes a signal issue
- Guoqing Jiang fixes a typo in FailFast test
- Other trival fixes

Please note, there is a merge conflict with block tree and causes build error,
which could be fixed by this one:

diff --git a/drivers/md/md.c b/drivers/md/md.c
index c4d41b03324b..8cdca0296749 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5450,7 +5450,7 @@ int md_run(struct mddev *mddev)
return -ENOMEM;
}
if (mddev->sync_set == NULL) {
-   mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0);
+   mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0, 
BIOSET_NEED_BVECS);
if (!mddev->sync_set)
return -ENOMEM;
}

Thanks,
Shaohua


The following changes since commit 63f700aab4c11d46626de3cd051dae56cf7e9056:

  Merge tag 'xtensa-20170612' of git://github.com/jcmvbkbc/linux-xtensa 
(2017-06-13 15:09:10 +0900)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/shli/md.git for-next

for you to fetch changes up to 7184ef8bab0cb865c3cea9dd1a675771145df0af:

  MD: fix sleep in atomic (2017-07-03 14:38:59 -0700)


Guoqing Jiang (2):
  md/raid10: fix FailFast test for wrong device
  md/raid1: remove unused bio in sync_request_write

Lidong Zhong (1):
  md: change the initialization value for a spare device spot to 
MD_DISK_ROLE_SPARE

Mikulas Patocka (1):
  md: don't use flush_signals in userspace processes

NeilBrown (2):
  md: fix deadlock between mddev_suspend() and md_write_start()
  md: use a separate bio_set for synchronous IO.

Shaohua Li (2):
  MD: fix a null dereference
  MD: fix sleep in atomic

 drivers/md/faulty.c|  5 +++--
 drivers/md/linear.c|  7 ---
 drivers/md/md.c| 47 ++-
 drivers/md/md.h|  7 +--
 drivers/md/multipath.c |  8 
 drivers/md/raid0.c |  7 ---
 drivers/md/raid1.c | 20 
 drivers/md/raid10.c| 16 +---
 drivers/md/raid5.c | 22 +-
 9 files changed, 92 insertions(+), 47 deletions(-)

[GIT PULL] MD update for 4.13

[Shaohua Li]

Hi,
Please pull MD update for 4.13. This update only includes several bug fixes:

- Neil Brown fixes a deadlock in MD suspend and a potential bug in bio 
allocation.
- Mikulas Patocka fixes a signal issue
- Guoqing Jiang fixes a typo in FailFast test
- Other trival fixes

Please note, there is a merge conflict with block tree and causes build error,
which could be fixed by this one:

diff --git a/drivers/md/md.c b/drivers/md/md.c
index c4d41b03324b..8cdca0296749 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5450,7 +5450,7 @@ int md_run(struct mddev *mddev)
return -ENOMEM;
}
if (mddev->sync_set == NULL) {
-   mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0);
+   mddev->sync_set = bioset_create(BIO_POOL_SIZE, 0, 
BIOSET_NEED_BVECS);
if (!mddev->sync_set)
return -ENOMEM;
}

Thanks,
Shaohua


The following changes since commit 63f700aab4c11d46626de3cd051dae56cf7e9056:

  Merge tag 'xtensa-20170612' of git://github.com/jcmvbkbc/linux-xtensa 
(2017-06-13 15:09:10 +0900)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/shli/md.git for-next

for you to fetch changes up to 7184ef8bab0cb865c3cea9dd1a675771145df0af:

  MD: fix sleep in atomic (2017-07-03 14:38:59 -0700)


Guoqing Jiang (2):
  md/raid10: fix FailFast test for wrong device
  md/raid1: remove unused bio in sync_request_write

Lidong Zhong (1):
  md: change the initialization value for a spare device spot to 
MD_DISK_ROLE_SPARE

Mikulas Patocka (1):
  md: don't use flush_signals in userspace processes

NeilBrown (2):
  md: fix deadlock between mddev_suspend() and md_write_start()
  md: use a separate bio_set for synchronous IO.

Shaohua Li (2):
  MD: fix a null dereference
  MD: fix sleep in atomic

 drivers/md/faulty.c|  5 +++--
 drivers/md/linear.c|  7 ---
 drivers/md/md.c| 47 ++-
 drivers/md/md.h|  7 +--
 drivers/md/multipath.c |  8 
 drivers/md/raid0.c |  7 ---
 drivers/md/raid1.c | 20 
 drivers/md/raid10.c| 16 +---
 drivers/md/raid5.c | 22 +-
 9 files changed, 92 insertions(+), 47 deletions(-)

[PATCH] auxdisplay: cfag12864bfb: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/auxdisplay/cfag12864bfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/cfag12864bfb.c 
b/drivers/auxdisplay/cfag12864bfb.c
index a387403..dcbee7e 100644
--- a/drivers/auxdisplay/cfag12864bfb.c
+++ b/drivers/auxdisplay/cfag12864bfb.c
@@ -37,7 +37,7 @@
 
 #define CFAG12864BFB_NAME "cfag12864bfb"
 
-static struct fb_fix_screeninfo cfag12864bfb_fix = {
+static const struct fb_fix_screeninfo cfag12864bfb_fix = {
.id = "cfag12864b",
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_MONO10,
@@ -48,7 +48,7 @@ static struct fb_fix_screeninfo cfag12864bfb_fix = {
.accel = FB_ACCEL_NONE,
 };
 
-static struct fb_var_screeninfo cfag12864bfb_var = {
+static const struct fb_var_screeninfo cfag12864bfb_var = {
.xres = CFAG12864B_WIDTH,
.yres = CFAG12864B_HEIGHT,
.xres_virtual = CFAG12864B_WIDTH,
-- 
2.5.0

[PATCH] auxdisplay: cfag12864bfb: constify fb_fix_screeninfo and fb_var_screeninfo structures

[Gustavo A. R. Silva]

These structures are only used to copy into other structures,
so declare them as const.

This issue was detected using Coccinelle and the following semantic patch:

@r disable optional_qualifier@
identifier i;
position p;
@@
static struct fb_fix_screeninfo i@p = { ... };

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct fb_fix_screeninfo e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct fb_fix_screeninfo i = { ... };

The semantic patch for fb_var_screeninfo is analogous.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/auxdisplay/cfag12864bfb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/cfag12864bfb.c 
b/drivers/auxdisplay/cfag12864bfb.c
index a387403..dcbee7e 100644
--- a/drivers/auxdisplay/cfag12864bfb.c
+++ b/drivers/auxdisplay/cfag12864bfb.c
@@ -37,7 +37,7 @@
 
 #define CFAG12864BFB_NAME "cfag12864bfb"
 
-static struct fb_fix_screeninfo cfag12864bfb_fix = {
+static const struct fb_fix_screeninfo cfag12864bfb_fix = {
.id = "cfag12864b",
.type = FB_TYPE_PACKED_PIXELS,
.visual = FB_VISUAL_MONO10,
@@ -48,7 +48,7 @@ static struct fb_fix_screeninfo cfag12864bfb_fix = {
.accel = FB_ACCEL_NONE,
 };
 
-static struct fb_var_screeninfo cfag12864bfb_var = {
+static const struct fb_var_screeninfo cfag12864bfb_var = {
.xres = CFAG12864B_WIDTH,
.yres = CFAG12864B_HEIGHT,
.xres_virtual = CFAG12864B_WIDTH,
-- 
2.5.0