[PATCH] tpm: invalid self test error message

[Jarkko Sakkinen]

The driver emits invalid self test error message even though the init
succeeds.

Signed-off-by: Jarkko Sakkinen 
Fixes: cae8b441fc20 ("tpm: Factor out common startup code")
---
 drivers/char/tpm/tpm2-cmd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 955fd26..906c285 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -966,7 +966,7 @@ int tpm2_auto_startup(struct tpm_chip *chip)
goto out;
 
rc = tpm2_do_selftest(chip);
-   if (rc != TPM2_RC_INITIALIZE) {
+   if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
dev_err(>dev, "TPM self test failed\n");
goto out;
}
@@ -983,7 +983,6 @@ int tpm2_auto_startup(struct tpm_chip *chip)
}
}
 
-   return rc;
 out:
if (rc > 0)
rc = -ENODEV;
-- 
2.7.4

Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip

[Michal Simek]

On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
> The Xilinx AXI Interrupt Controller IP block is used by the MIPS
> based xilfpga platform.
> 
> Move the interrupt controller code out of arch/microblaze so that
> it can be used by everyone

if this is just move that you should setup your git right that it won't
be remove on one side and add on other side.

It should look like this when git mv is used and git setup is right.

diff --git a/Makefile b/Makefile2
similarity index 100%
rename from Makefile
rename to Makefile2

Thanks,
Michal

[PATCH] tpm: invalid self test error message

[Jarkko Sakkinen]

The driver emits invalid self test error message even though the init
succeeds.

Signed-off-by: Jarkko Sakkinen 
Fixes: cae8b441fc20 ("tpm: Factor out common startup code")
---
 drivers/char/tpm/tpm2-cmd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 955fd26..906c285 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -966,7 +966,7 @@ int tpm2_auto_startup(struct tpm_chip *chip)
goto out;
 
rc = tpm2_do_selftest(chip);
-   if (rc != TPM2_RC_INITIALIZE) {
+   if (rc != 0 && rc != TPM2_RC_INITIALIZE) {
dev_err(>dev, "TPM self test failed\n");
goto out;
}
@@ -983,7 +983,6 @@ int tpm2_auto_startup(struct tpm_chip *chip)
}
}
 
-   return rc;
 out:
if (rc > 0)
rc = -ENODEV;
-- 
2.7.4

Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip

[Michal Simek]

On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
> The Xilinx AXI Interrupt Controller IP block is used by the MIPS
> based xilfpga platform.
> 
> Move the interrupt controller code out of arch/microblaze so that
> it can be used by everyone

if this is just move that you should setup your git right that it won't
be remove on one side and add on other side.

It should look like this when git mv is used and git setup is right.

diff --git a/Makefile b/Makefile2
similarity index 100%
rename from Makefile
rename to Makefile2

Thanks,
Michal

Re: [PATCH net-next] net: dsa: remove ds_to_priv

[David Miller]

From: Vivien Didelot 
Date: Wed, 31 Aug 2016 18:06:13 -0400

> Access the priv member of the dsa_switch structure directly, instead of
> having an unnecessary helper.
> 
> Signed-off-by: Vivien Didelot 

Applied.

Re: [PATCH net-next] net: dsa: remove ds_to_priv

[David Miller]

From: Vivien Didelot 
Date: Wed, 31 Aug 2016 18:06:13 -0400

> Access the priv member of the dsa_switch structure directly, instead of
> having an unnecessary helper.
> 
> Signed-off-by: Vivien Didelot 

Applied.

Re: [PATCH v2 net] rps: flow_dissector: Fix uninitialized flow_keys used in __skb_get_hash possibly

[David Miller]

From: f...@ikuai8.com
Date: Wed, 31 Aug 2016 14:15:05 +0800

> From: Gao Feng 
> 
> The original codes depend on that the function parameters are evaluated from
> left to right. But the parameter's evaluation order is not defined in C
> standard actually.
> 
> When flow_keys_have_l4() is invoked before ___skb_get_hash(skb, ,
> hashrnd) with some compilers or environment, the keys passed to
> flow_keys_have_l4 is not initialized.
> 
> Fixes: 6db61d79c1e1 ("flow_dissector: Ignore flow dissector return value from 
> ___skb_get_hash")
> 
> Acked-by: Eric Dumazet 
> Signed-off-by: Gao Feng 

Applied.

Re: [PATCH v2 net] rps: flow_dissector: Fix uninitialized flow_keys used in __skb_get_hash possibly

[David Miller]

From: f...@ikuai8.com
Date: Wed, 31 Aug 2016 14:15:05 +0800

> From: Gao Feng 
> 
> The original codes depend on that the function parameters are evaluated from
> left to right. But the parameter's evaluation order is not defined in C
> standard actually.
> 
> When flow_keys_have_l4() is invoked before ___skb_get_hash(skb, ,
> hashrnd) with some compilers or environment, the keys passed to
> flow_keys_have_l4 is not initialized.
> 
> Fixes: 6db61d79c1e1 ("flow_dissector: Ignore flow dissector return value from 
> ___skb_get_hash")
> 
> Acked-by: Eric Dumazet 
> Signed-off-by: Gao Feng 

Applied.

[PATCH] x86/AMD: Apply erratum 665 on machines without a BIOS fix

[Borislav Petkov]

From: Emanuel Czirai 

AMD F12h machines have an erratum which can cause DIV/IDIV to behave
unpredictably. The workaround is to set MSRC001_1029[31] but sometimes
there is no BIOS update containing that workaround so let's do it
ourselves in that case. It is simple enough.

Signed-off-by: Emanuel Czirai 
Cc: Yaowu Xu 
[ Write commit message. ]
Signed-off-by: Borislav Petkov 
---
 arch/x86/kernel/cpu/amd.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index f5c69d8974e1..eaf938e9df88 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -669,6 +669,14 @@ static void init_amd_gh(struct cpuinfo_x86 *c)
set_cpu_bug(c, X86_BUG_AMD_TLB_MMATCH);
 }
 
+static void init_amd_ln(struct cpuinfo_x86 *c)
+{
+#define MSR_AMD64_DE_CFG   0xC0011029
+
+   /* Apply erratum 665 fix to machines without a BIOS fix.  */
+   msr_set_bit(MSR_AMD64_DE_CFG, 31);
+}
+
 static void init_amd_bd(struct cpuinfo_x86 *c)
 {
u64 value;
@@ -726,6 +734,7 @@ static void init_amd(struct cpuinfo_x86 *c)
case 6:init_amd_k7(c); break;
case 0xf:  init_amd_k8(c); break;
case 0x10: init_amd_gh(c); break;
+   case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
}
 
-- 
2.8.4

[PATCH] x86/AMD: Apply erratum 665 on machines without a BIOS fix

[Borislav Petkov]

From: Emanuel Czirai 

AMD F12h machines have an erratum which can cause DIV/IDIV to behave
unpredictably. The workaround is to set MSRC001_1029[31] but sometimes
there is no BIOS update containing that workaround so let's do it
ourselves in that case. It is simple enough.

Signed-off-by: Emanuel Czirai 
Cc: Yaowu Xu 
[ Write commit message. ]
Signed-off-by: Borislav Petkov 
---
 arch/x86/kernel/cpu/amd.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index f5c69d8974e1..eaf938e9df88 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -669,6 +669,14 @@ static void init_amd_gh(struct cpuinfo_x86 *c)
set_cpu_bug(c, X86_BUG_AMD_TLB_MMATCH);
 }
 
+static void init_amd_ln(struct cpuinfo_x86 *c)
+{
+#define MSR_AMD64_DE_CFG   0xC0011029
+
+   /* Apply erratum 665 fix to machines without a BIOS fix.  */
+   msr_set_bit(MSR_AMD64_DE_CFG, 31);
+}
+
 static void init_amd_bd(struct cpuinfo_x86 *c)
 {
u64 value;
@@ -726,6 +734,7 @@ static void init_amd(struct cpuinfo_x86 *c)
case 6:init_amd_k7(c); break;
case 0xf:  init_amd_k8(c); break;
case 0x10: init_amd_gh(c); break;
+   case 0x12: init_amd_ln(c); break;
case 0x15: init_amd_bd(c); break;
}
 
-- 
2.8.4

Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver

[Andi Shyti]

Hi Rob,

> > Document the ir-spi driver's binding which is a IR led driven
> > through the SPI line.
> >
> > Signed-off-by: Andi Shyti 
> > ---
> >  Documentation/devicetree/bindings/media/spi-ir.txt | 26 
> > ++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt 
> > b/Documentation/devicetree/bindings/media/spi-ir.txt
> > new file mode 100644
> > index 000..85cb21b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> 
> Move this to bindings/leds, and CC the leds maintainers.

More than an LED this is the driver of a remote controller, the
driver itself is under drivers/media/rc/.

Besides all the transmitters have an LED but still they are media
devices. This is a bit special because it's so simple that the
only hardware left is the LED itself, but still it's a media remote
controller.

> > @@ -0,0 +1,26 @@
> > +Device tree bindings for IR LED connected through SPI bus which is used as
> > +remote controller.
> > +
> > +The IR LED switch is connected to the MOSI line of the SPI device and the 
> > data
> > +are delivered thourgh that.
> > +
> > +Required properties:
> > +   - compatible: should be "ir-spi".
> 
> Really this is just an LED connected to a SPI, so maybe this should
> just be "spi-led". If being more specific is helpful, then I'm all for
> that, but perhaps spi-ir-led. (Trying to be consistent in naming with
> gpio-leds).

As I mentioned above, all transmitters have an LED, but they do
not have the 'led' name. "ir-spi" is coherent with the device
driver name and the driver name is coherent with the media/rc
driver's naming.

> > +
> > +Optional properties:
> > +   - irled,switch: specifies the gpio switch which enables the irled/
> 
> As I said previously, "switch-gpios" as gpio lines should have a
> '-gpios' suffix. Or better yet, "enable-gpios" as that is a standard
> name for an enable line.

OK, thanks!

> > +   - negated: boolean value that specifies whether the output is 
> > negated
> > + with a NOT gate.
> 
> Negated or inverted assumes I know what normal is. Define this in
> terms of what is the on state. If on is normally active low, then this
> should be led-active-high. There may already be an LED property for
> this.

Yes, thanks!

> > +   - duty-cycle: 8 bit value that stores the percentage of the duty 
> > cycle.
> > + it can be 50, 60, 70, 75, 80 or 90.
> 
> This is percent time on or off?

Will add more details, thanks.

If it's OK for you, I would keep the name and documentation path
and fix the rest. Please let me know if I'm missing something :)

Thanks,
Andi

Re: [PATCH v2 6/7] Documentation: bindings: add documentation for ir-spi device driver

[Andi Shyti]

Hi Rob,

> > Document the ir-spi driver's binding which is a IR led driven
> > through the SPI line.
> >
> > Signed-off-by: Andi Shyti 
> > ---
> >  Documentation/devicetree/bindings/media/spi-ir.txt | 26 
> > ++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/spi-ir.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/spi-ir.txt 
> > b/Documentation/devicetree/bindings/media/spi-ir.txt
> > new file mode 100644
> > index 000..85cb21b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/spi-ir.txt
> 
> Move this to bindings/leds, and CC the leds maintainers.

More than an LED this is the driver of a remote controller, the
driver itself is under drivers/media/rc/.

Besides all the transmitters have an LED but still they are media
devices. This is a bit special because it's so simple that the
only hardware left is the LED itself, but still it's a media remote
controller.

> > @@ -0,0 +1,26 @@
> > +Device tree bindings for IR LED connected through SPI bus which is used as
> > +remote controller.
> > +
> > +The IR LED switch is connected to the MOSI line of the SPI device and the 
> > data
> > +are delivered thourgh that.
> > +
> > +Required properties:
> > +   - compatible: should be "ir-spi".
> 
> Really this is just an LED connected to a SPI, so maybe this should
> just be "spi-led". If being more specific is helpful, then I'm all for
> that, but perhaps spi-ir-led. (Trying to be consistent in naming with
> gpio-leds).

As I mentioned above, all transmitters have an LED, but they do
not have the 'led' name. "ir-spi" is coherent with the device
driver name and the driver name is coherent with the media/rc
driver's naming.

> > +
> > +Optional properties:
> > +   - irled,switch: specifies the gpio switch which enables the irled/
> 
> As I said previously, "switch-gpios" as gpio lines should have a
> '-gpios' suffix. Or better yet, "enable-gpios" as that is a standard
> name for an enable line.

OK, thanks!

> > +   - negated: boolean value that specifies whether the output is 
> > negated
> > + with a NOT gate.
> 
> Negated or inverted assumes I know what normal is. Define this in
> terms of what is the on state. If on is normally active low, then this
> should be led-active-high. There may already be an LED property for
> this.

Yes, thanks!

> > +   - duty-cycle: 8 bit value that stores the percentage of the duty 
> > cycle.
> > + it can be 50, 60, 70, 75, 80 or 90.
> 
> This is percent time on or off?

Will add more details, thanks.

If it's OK for you, I would keep the name and documentation path
and fix the rest. Please let me know if I'm missing something :)

Thanks,
Andi

Re: [PATCH] staging: i4l: act2000: Add blank line after declaration

[Joe Perches]

On Fri, 2016-09-02 at 01:09 -0400, Anson Jacob wrote:
> Fix checkpatch.pl warning:
> Missing a blank line after declarations
[]
> diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
> b/drivers/staging/i4l/act2000/act2000_isa.c
[]
> @@ -259,6 +259,7 @@ act2000_isa_receive(act2000_card *card)
>      "act2000_isa_receive: Invalid 
> CAPI msg\n");
>   {
>   int i; __u8 *p; __u8 *t; __u8 
> tmp[30];
> +
>   for (i = 0, p = (__u8 
> *)>idat.isa.rcvhdr, t = tmp; i < 8; i++)
>   t += sprintf(t, "%02x 
> ", *(p++));
>   printk(KERN_WARNING 
> "act2000_isa_receive: %s\n", tmp);

It'd be better to use the %*ph vsprintf extension
(see Documentation/printk-formats.txt) without
any of the loop code and variable declarations:

pr_warn("%s: %8ph\n",
__func__, 
card->idat.isa.rcvhdr);
or maybe
pr_warn("%s: %*ph\n",
__func__,

(int)ARRAY_SIZE(card->idat.isa.rcvhdr),
card->idat.isa.rcvhdr);

Re: [PATCH] staging: i4l: act2000: Add blank line after declaration

[Joe Perches]

On Fri, 2016-09-02 at 01:09 -0400, Anson Jacob wrote:
> Fix checkpatch.pl warning:
> Missing a blank line after declarations
[]
> diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
> b/drivers/staging/i4l/act2000/act2000_isa.c
[]
> @@ -259,6 +259,7 @@ act2000_isa_receive(act2000_card *card)
>      "act2000_isa_receive: Invalid 
> CAPI msg\n");
>   {
>   int i; __u8 *p; __u8 *t; __u8 
> tmp[30];
> +
>   for (i = 0, p = (__u8 
> *)>idat.isa.rcvhdr, t = tmp; i < 8; i++)
>   t += sprintf(t, "%02x 
> ", *(p++));
>   printk(KERN_WARNING 
> "act2000_isa_receive: %s\n", tmp);

It'd be better to use the %*ph vsprintf extension
(see Documentation/printk-formats.txt) without
any of the loop code and variable declarations:

pr_warn("%s: %8ph\n",
__func__, 
card->idat.isa.rcvhdr);
or maybe
pr_warn("%s: %*ph\n",
__func__,

(int)ARRAY_SIZE(card->idat.isa.rcvhdr),
card->idat.isa.rcvhdr);

Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI

[Andi Shyti]

> Thanks Andi, this is looking great!

Thanks Sean! With your reviews the whole thing looks much better
now :)

I agree with all your points here, I will fix them. Can I add
your reviewd-by?

Thanks,
Andi

Re: [PATCH v2 7/7] [media] rc: add support for IR LEDs driven through SPI

[Andi Shyti]

> Thanks Andi, this is looking great!

Thanks Sean! With your reviews the whole thing looks much better
now :)

I agree with all your points here, I will fix them. Can I add
your reviewd-by?

Thanks,
Andi

Re: MAINTAINERS without commits in the last 3 years

[Borislav Petkov]

On Thu, Sep 01, 2016 at 09:02:29AM -0700, Joe Perches wrote:
> If you want to apply this one first, that's fine by me.

Thanks, I'll route it through the EDAC tree.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: MAINTAINERS without commits in the last 3 years

[Borislav Petkov]

On Thu, Sep 01, 2016 at 09:02:29AM -0700, Joe Perches wrote:
> If you want to apply this one first, that's fine by me.

Thanks, I'll route it through the EDAC tree.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation

[Andi Shyti]

Hi Sean,

> > ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -   dev = rc_allocate_device();
> > +   dev = rc_allocate_device(RC_DRIVER_IR_RAW);
> > if (!ir || !dev)
> > goto err_out_free;
> >  
> 
> If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.
> 
> 
> > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev 
> > *pci)
> > dev->scancode_mask = hardware_mask;
> >  
> > if (ir->sampling) {
> > -   dev->driver_type = RC_DRIVER_IR_RAW;
> > dev->timeout = 10 * 1000 * 1000; /* 10 ms */
> > } else {
> > dev->driver_type = RC_DRIVER_SCANCODE;
> 
> That assignment shouldn't really be there any more.

I think this doesn't change the driver's behavior, because I
either do like:

  -   dev = rc_allocate_device();
  +   dev = rc_allocate_device(RC_DRIVER_SCANCODE);

  [ ... ]

  if (ir->sampling) {
  dev->driver_type = RC_DRIVER_IR_RAW;
  dev->timeout = 10 * 1000 * 1000; /* 10 ms */
  } else {
   -  dev->driver_type = RC_DRIVER_SCANCODE;

Or I would need to do aftr the long switch...case statement

+  if (ir->sampling) {
+  dev = rc_allocate_device(RC_DRIVER_IR_RAW);
+  ...
+  } else {
+  dev = rc_allocate_device(RC_DRIVER_SCANCODE);
+  ...

I prefered the first way because it doesn't alter much the
driver.

> > ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -   rc = rc_allocate_device();
> > +   rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > if (!ir || !rc) {
> > err = -ENOMEM;
> > goto err_out_free;
> 
> This is not correct, I'm afraid. If you look at the code you can see that
> if raw_decode is true, then it should be RC_DRIVER_IR_RAW.

Same here, the driver doesn't change the behavior. raw_decode can
be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE
and depending on value of raw_decode it's chaged to
RC_DRIVER_IR_RAW.

also in this case I can do

+  if (raw_decode) {
+  rc = rc_allocate_device(RC_DRIVER_IR_RAW);
+  ...
+  } else {
+  rc = rc_allocate_device(RC_DRIVER_SCANCODE);
+  ...

but also in this case my original approach doesn't add much
changes.

Thanks,
Andi

Re: [PATCH v2 1/7] [media] rc-main: assign driver type during allocation

[Andi Shyti]

Hi Sean,

> > ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -   dev = rc_allocate_device();
> > +   dev = rc_allocate_device(RC_DRIVER_IR_RAW);
> > if (!ir || !dev)
> > goto err_out_free;
> >  
> 
> If ir->sampling = 0 then it should be RC_DRIVER_SCANCODE.
> 
> 
> > @@ -481,7 +481,6 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev 
> > *pci)
> > dev->scancode_mask = hardware_mask;
> >  
> > if (ir->sampling) {
> > -   dev->driver_type = RC_DRIVER_IR_RAW;
> > dev->timeout = 10 * 1000 * 1000; /* 10 ms */
> > } else {
> > dev->driver_type = RC_DRIVER_SCANCODE;
> 
> That assignment shouldn't really be there any more.

I think this doesn't change the driver's behavior, because I
either do like:

  -   dev = rc_allocate_device();
  +   dev = rc_allocate_device(RC_DRIVER_SCANCODE);

  [ ... ]

  if (ir->sampling) {
  dev->driver_type = RC_DRIVER_IR_RAW;
  dev->timeout = 10 * 1000 * 1000; /* 10 ms */
  } else {
   -  dev->driver_type = RC_DRIVER_SCANCODE;

Or I would need to do aftr the long switch...case statement

+  if (ir->sampling) {
+  dev = rc_allocate_device(RC_DRIVER_IR_RAW);
+  ...
+  } else {
+  dev = rc_allocate_device(RC_DRIVER_SCANCODE);
+  ...

I prefered the first way because it doesn't alter much the
driver.

> > ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> > -   rc = rc_allocate_device();
> > +   rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> > if (!ir || !rc) {
> > err = -ENOMEM;
> > goto err_out_free;
> 
> This is not correct, I'm afraid. If you look at the code you can see that
> if raw_decode is true, then it should be RC_DRIVER_IR_RAW.

Same here, the driver doesn't change the behavior. raw_decode can
be both 'true' or 'false' it's set as default RC_DRIVER_SCANCODE
and depending on value of raw_decode it's chaged to
RC_DRIVER_IR_RAW.

also in this case I can do

+  if (raw_decode) {
+  rc = rc_allocate_device(RC_DRIVER_IR_RAW);
+  ...
+  } else {
+  rc = rc_allocate_device(RC_DRIVER_SCANCODE);
+  ...

but also in this case my original approach doesn't add much
changes.

Thanks,
Andi

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 7:35 PM, Ziyuan Xu  wrote:
>
>
> On 2016年09月02日 05:29, Doug Anderson wrote:
>>
>> Hi,
>>
>> On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu  wrote:
>>>
>>> Hi
>>>
>>>
>>> On 2016年09月01日 12:20, Doug Anderson wrote:

 Hi,

 On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu 
 wrote:
>>
>> This is fine to pick up _only_ if you don't care about suspend/resume.
>> If you care about suspend/resume then someone needs to first write a
>> patch that will re-init all "corecfg" values after power is turned on.
>
>
> Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
> don't need to strore/re-init it after resume.
> corecfg_clockmultiplier is only used to fetch host->clk_mul, and
> host->clk_mul has been a fixed value at run-time, unless driver unbind.
> The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to
> check
> the xin_clk at probe time, we don't reference it at run-time.
> BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
> works
> fine.

 I guess I don't actually know how the corecfg_clockmultiplier and
 corecfg_baseclkfreq fields are actually used, but I presume that they
 actually do something useful and aren't used to just communicate back
 to software?
>>>
>>>
>>> Take corecfg_clockmultiplier as example.
>>> 1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
>>> 2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're
>>> used
>>> for further initialization.
>>> 3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
>>> frequency to play.
>>>
>>> I think we don't need to store it due to it's a fixed value at run-time,
>>> even if it is reset after a power cycle, the above will not be changed
>>> via
>>> software, except for dirver unbind .
>>>
 I know that:

 1. If I don't pick this patch and I suspend/resume,
 corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
 suspend / resume.

 2. If I do pick this patch and I suspend/resume,
 corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
 suspend/resume (tested by reading /dev/mem directly from userspace
 after suspend/resume).


 Are you saying that it is unimportant that corecfg_clockmultiplier and
 corecfg_baseclkfreq are wrong?
>>>
>>>
>>> Yup, corecfg_* stuff will be reset after a power cycle.
>>> I mean that we need only to guarantee they're correct at probe time.
>>
>> So are you saying that the entire purpose of "corecfg_clockmultiplier"
>> is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
>> register to get a certain value?
>> ...and that the entire purpose of "corecfg_baseclkfreq" is that it
>> causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
>> to get a certain value?
>
> Yes, on rk3399:
> corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
> corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock
>
> If you re-write to either corecfg_* stuff,  the corresponding CAP register
> field will be changed too.
> sdhci driver will fetch CAP register for initialization, we only need to
> guarantee they're correct at probe time.
>
> Did that all make sense?

Yes.  Very odd, but it makes sense.

It would still be nice to get these restored after runtime resume just
for cleanliness, but it's not a blocker IMHO.

Reviewed-by: Douglas Anderson 

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 7:35 PM, Ziyuan Xu  wrote:
>
>
> On 2016年09月02日 05:29, Doug Anderson wrote:
>>
>> Hi,
>>
>> On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu  wrote:
>>>
>>> Hi
>>>
>>>
>>> On 2016年09月01日 12:20, Doug Anderson wrote:

 Hi,

 On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu 
 wrote:
>>
>> This is fine to pick up _only_ if you don't care about suspend/resume.
>> If you care about suspend/resume then someone needs to first write a
>> patch that will re-init all "corecfg" values after power is turned on.
>
>
> Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
> don't need to strore/re-init it after resume.
> corecfg_clockmultiplier is only used to fetch host->clk_mul, and
> host->clk_mul has been a fixed value at run-time, unless driver unbind.
> The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to
> check
> the xin_clk at probe time, we don't reference it at run-time.
> BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
> works
> fine.

 I guess I don't actually know how the corecfg_clockmultiplier and
 corecfg_baseclkfreq fields are actually used, but I presume that they
 actually do something useful and aren't used to just communicate back
 to software?
>>>
>>>
>>> Take corecfg_clockmultiplier as example.
>>> 1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
>>> 2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're
>>> used
>>> for further initialization.
>>> 3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
>>> frequency to play.
>>>
>>> I think we don't need to store it due to it's a fixed value at run-time,
>>> even if it is reset after a power cycle, the above will not be changed
>>> via
>>> software, except for dirver unbind .
>>>
 I know that:

 1. If I don't pick this patch and I suspend/resume,
 corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
 suspend / resume.

 2. If I do pick this patch and I suspend/resume,
 corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
 suspend/resume (tested by reading /dev/mem directly from userspace
 after suspend/resume).


 Are you saying that it is unimportant that corecfg_clockmultiplier and
 corecfg_baseclkfreq are wrong?
>>>
>>>
>>> Yup, corecfg_* stuff will be reset after a power cycle.
>>> I mean that we need only to guarantee they're correct at probe time.
>>
>> So are you saying that the entire purpose of "corecfg_clockmultiplier"
>> is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
>> register to get a certain value?
>> ...and that the entire purpose of "corecfg_baseclkfreq" is that it
>> causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
>> to get a certain value?
>
> Yes, on rk3399:
> corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
> corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock
>
> If you re-write to either corecfg_* stuff,  the corresponding CAP register
> field will be changed too.
> sdhci driver will fetch CAP register for initialization, we only need to
> guarantee they're correct at probe time.
>
> Did that all make sense?

Yes.  Very odd, but it makes sense.

It would still be nice to get these restored after runtime resume just
for cleanliness, but it's not a blocker IMHO.

Reviewed-by: Douglas Anderson 

[PATCH] staging: i4l: act2000: Add blank line after declaration

[Anson Jacob]

Fix checkpatch.pl warning:
Missing a blank line after declarations

Signed-off-by: Anson Jacob 
---
 drivers/staging/i4l/act2000/act2000_isa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
b/drivers/staging/i4l/act2000/act2000_isa.c
index 1d93151..fbb15f5 100644
--- a/drivers/staging/i4l/act2000/act2000_isa.c
+++ b/drivers/staging/i4l/act2000/act2000_isa.c
@@ -259,6 +259,7 @@ act2000_isa_receive(act2000_card *card)
   "act2000_isa_receive: Invalid 
CAPI msg\n");
{
int i; __u8 *p; __u8 *t; __u8 
tmp[30];
+
for (i = 0, p = (__u8 
*)>idat.isa.rcvhdr, t = tmp; i < 8; i++)
t += sprintf(t, "%02x 
", *(p++));
printk(KERN_WARNING 
"act2000_isa_receive: %s\n", tmp);
-- 
2.7.4

[PATCH] staging: i4l: act2000: Add blank line after declaration

[Anson Jacob]

Fix checkpatch.pl warning:
Missing a blank line after declarations

Signed-off-by: Anson Jacob 
---
 drivers/staging/i4l/act2000/act2000_isa.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
b/drivers/staging/i4l/act2000/act2000_isa.c
index 1d93151..fbb15f5 100644
--- a/drivers/staging/i4l/act2000/act2000_isa.c
+++ b/drivers/staging/i4l/act2000/act2000_isa.c
@@ -259,6 +259,7 @@ act2000_isa_receive(act2000_card *card)
   "act2000_isa_receive: Invalid 
CAPI msg\n");
{
int i; __u8 *p; __u8 *t; __u8 
tmp[30];
+
for (i = 0, p = (__u8 
*)>idat.isa.rcvhdr, t = tmp; i < 8; i++)
t += sprintf(t, "%02x 
", *(p++));
printk(KERN_WARNING 
"act2000_isa_receive: %s\n", tmp);
-- 
2.7.4

[PATCH] staging: i4l: act2000: Remove braces for single statement

[Anson Jacob]

Fix checkpatch.pl warning:
braces {} are not necessary for single statement blocks

Signed-off-by: Anson Jacob 
---
 drivers/staging/i4l/act2000/act2000_isa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
b/drivers/staging/i4l/act2000/act2000_isa.c
index f0eb844..1d93151 100644
--- a/drivers/staging/i4l/act2000/act2000_isa.c
+++ b/drivers/staging/i4l/act2000/act2000_isa.c
@@ -134,9 +134,9 @@ act2000_isa_config_irq(act2000_card *card, short irq)
 {
int old_irq;
 
-   if (card->flags & ACT2000_FLAGS_IVALID) {
+   if (card->flags & ACT2000_FLAGS_IVALID)
free_irq(card->irq, card);
-   }
+
card->flags &= ~ACT2000_FLAGS_IVALID;
outb(ISA_COR_IRQOFF, ISA_PORT_COR);
if (!irq)
-- 
2.7.4

[PATCH] staging: i4l: act2000: Remove braces for single statement

[Anson Jacob]

Fix checkpatch.pl warning:
braces {} are not necessary for single statement blocks

Signed-off-by: Anson Jacob 
---
 drivers/staging/i4l/act2000/act2000_isa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/i4l/act2000/act2000_isa.c 
b/drivers/staging/i4l/act2000/act2000_isa.c
index f0eb844..1d93151 100644
--- a/drivers/staging/i4l/act2000/act2000_isa.c
+++ b/drivers/staging/i4l/act2000/act2000_isa.c
@@ -134,9 +134,9 @@ act2000_isa_config_irq(act2000_card *card, short irq)
 {
int old_irq;
 
-   if (card->flags & ACT2000_FLAGS_IVALID) {
+   if (card->flags & ACT2000_FLAGS_IVALID)
free_irq(card->irq, card);
-   }
+
card->flags &= ~ACT2000_FLAGS_IVALID;
outb(ISA_COR_IRQOFF, ISA_PORT_COR);
if (!irq)
-- 
2.7.4

[PATCH] Documentation: kasan: arm64 has kasan support too

[Laurentiu Tudor]

Mention that arm64 also has kasan support.

Signed-off-by: Laurentiu Tudor 
---
 Documentation/kasan.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
index 7dd95b3..c156934 100644
--- a/Documentation/kasan.txt
+++ b/Documentation/kasan.txt
@@ -12,7 +12,7 @@ KASAN uses compile-time instrumentation for checking every 
memory access,
 therefore you will need a GCC version 4.9.2 or later. GCC 5.0 or later is
 required for detection of out-of-bounds accesses to stack or global variables.
 
-Currently KASAN is supported only for x86_64 architecture.
+Currently KASAN is supported only for x86_64 and arm64 architectures.
 
 1. Usage
 
-- 
1.8.3.1

bcachefs: Encryption (Posting for review)

[Kent Overstreet]

Encryption in bcachefs is done and working and I just finished documenting the
design - so now, it needs more eyeballs and vetting before letting users play
with it.

I'd appreciate help circulating this around to people who'd be qualified to
review it, too.

Also, bcachefs development is still moving along but it still needs funding: if
you're a fan of bcache or fancypants new filesystems, please chip in -
https://www.patreon.com/bcachefs
https://bcache.evilpiepirate.org/Bcachefs/

-

The master copy of this document is at:
https://bcache.evilpiepirate.org/Encryption/

# bcache/bcachefs encryption design:

This document is intended for review by cryptographers and other experience
implementers of cryptography code, before the design is frozen. Everything
described in this document has been implemented and tested.

The code may be found at
[[https://evilpiepirate.org/git/linux-bcache.git/log/?h=bcache-encryption]]

Feedback and comments should be sent to Kent Overstreet,
kent.overstr...@gmail.com.

## Intro:

Bcachefs provides whole-filesystem encryption, using ChaCha20/Poly1305.
Encryption may be enabled when creating a filesystem, or encryption may be
enabled on an existing filesystem (TODO: implement interface for enabling
encryption on an existing filesystem - kernel code exists).

Example:

$ bcache format --encrypted /dev/sda

(Enter passphrase when prompted)

$ bcache unlock /dev/sda

(Enter passphrase again)

Then mount as normal:

$ mount /dev/sda /mnt

## Goals:

Bcachefs encryption is meant to be a clean slate design that prioritizes
security and robustness, and is meant to defend against a wider variety of
yadversarial models than is typical in existing filesystem level or block level
encryption.

## Filesystem vs. directory encryption

We do not currently offer per directory encryption; instead, we take an "encrypt
everything" approach.

Rationale:

With per directory encryption, preventing metadata from leaking is highly
problematic.  For example, file sizes - file sizes are required for fsck, so
they would have to be stored unencrypted - or failing that some complicated way
of deferring fsck for that part of the filesystem until the key has been
provided. There would be additional complications around filenames, xattrs,
extents (and inline extents), etc. - not necessarily insurmountable, but they
would definitely lead to a more complicated, more fragile design.

With whole filesystem encryption, it’s much easier to say what is and isn’t
encrypted, we can encrypt at the granularity of an entire metadata write (a
journal entry, a btree node) and it's much easier to show that the contents -
everything after the header for that particular metadata write - will not leak.

### Algorithms

By virtue of working within a copy on write filesystem with provisions for ZFS
style checksums (that is, checksums with the pointers, not the data), we’re
able to use a modern AEAD style construction. We use ChaCha20 and Poly1305. We
use the cyphers directly instead of using the kernel AEAD library (and thus
means there's a bit more in the design that needs auditing).

The current design uses the same key for both ChaCha20 and Poly1305, but my
recent rereading of the Poly1305-AES paper seems to imply that the Poly1305 key
shouldn't be used for anything else. Guidance from actual cryptographers would
be appreciated here; the ChaCha20/Poly1305 AEAD RFC appears to be silent on the
matter.

Note that ChaCha20 is a stream cypher. This means that it’s critical that we use
a cryptographic MAC (which would be highly desirable anyways), and also avoiding
nonce reuse is critical. Getting nonces right is where most of the trickiness is
involved in bcachefs’s encryption.

The current algorithm choices are not hard coded. Bcachefs already has
selectable checksum types, and every individual data and metadata write has a
field that describes the checksum algorithm that was used. On disk, encrypted
data is represented as a new checksum type - so we now have [none, crc32c,
crc64, chacha20/poly1305] as possible methods for data to be
checksummed/encrypted. If in the future we add new encryption algorithms, users
will be able to switch to the new algorithm on existing encrypted filesystems;
new data will be written with the new algorithm and old data will be read with
the old algorithm until it is rewritten.

### Key derivation, master key

Userspace tooling takes the user's passphrase and derives an encryption key with
scrypt. This key is made available to the kernel (via the Linux kernel's keyring
service) prior to mounting the filesystem.

On filesystem mount, the userspace provided key is used to decrypt the master
key, which is stored in the superblock - also with ChaCha20. The master key is
encrypted with an 8 byte header, so that we can tell if the correct key was
supplied.

### Metadata

Except for the superblock, no metadata in bcache/bcachefs is updated in place -
everything is more or less log 

[PATCH] Documentation: kasan: arm64 has kasan support too

[Laurentiu Tudor]

Mention that arm64 also has kasan support.

Signed-off-by: Laurentiu Tudor 
---
 Documentation/kasan.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
index 7dd95b3..c156934 100644
--- a/Documentation/kasan.txt
+++ b/Documentation/kasan.txt
@@ -12,7 +12,7 @@ KASAN uses compile-time instrumentation for checking every 
memory access,
 therefore you will need a GCC version 4.9.2 or later. GCC 5.0 or later is
 required for detection of out-of-bounds accesses to stack or global variables.
 
-Currently KASAN is supported only for x86_64 architecture.
+Currently KASAN is supported only for x86_64 and arm64 architectures.
 
 1. Usage
 
-- 
1.8.3.1

bcachefs: Encryption (Posting for review)

[Kent Overstreet]

Encryption in bcachefs is done and working and I just finished documenting the
design - so now, it needs more eyeballs and vetting before letting users play
with it.

I'd appreciate help circulating this around to people who'd be qualified to
review it, too.

Also, bcachefs development is still moving along but it still needs funding: if
you're a fan of bcache or fancypants new filesystems, please chip in -
https://www.patreon.com/bcachefs
https://bcache.evilpiepirate.org/Bcachefs/

-

The master copy of this document is at:
https://bcache.evilpiepirate.org/Encryption/

# bcache/bcachefs encryption design:

This document is intended for review by cryptographers and other experience
implementers of cryptography code, before the design is frozen. Everything
described in this document has been implemented and tested.

The code may be found at
[[https://evilpiepirate.org/git/linux-bcache.git/log/?h=bcache-encryption]]

Feedback and comments should be sent to Kent Overstreet,
kent.overstr...@gmail.com.

## Intro:

Bcachefs provides whole-filesystem encryption, using ChaCha20/Poly1305.
Encryption may be enabled when creating a filesystem, or encryption may be
enabled on an existing filesystem (TODO: implement interface for enabling
encryption on an existing filesystem - kernel code exists).

Example:

$ bcache format --encrypted /dev/sda

(Enter passphrase when prompted)

$ bcache unlock /dev/sda

(Enter passphrase again)

Then mount as normal:

$ mount /dev/sda /mnt

## Goals:

Bcachefs encryption is meant to be a clean slate design that prioritizes
security and robustness, and is meant to defend against a wider variety of
yadversarial models than is typical in existing filesystem level or block level
encryption.

## Filesystem vs. directory encryption

We do not currently offer per directory encryption; instead, we take an "encrypt
everything" approach.

Rationale:

With per directory encryption, preventing metadata from leaking is highly
problematic.  For example, file sizes - file sizes are required for fsck, so
they would have to be stored unencrypted - or failing that some complicated way
of deferring fsck for that part of the filesystem until the key has been
provided. There would be additional complications around filenames, xattrs,
extents (and inline extents), etc. - not necessarily insurmountable, but they
would definitely lead to a more complicated, more fragile design.

With whole filesystem encryption, it’s much easier to say what is and isn’t
encrypted, we can encrypt at the granularity of an entire metadata write (a
journal entry, a btree node) and it's much easier to show that the contents -
everything after the header for that particular metadata write - will not leak.

### Algorithms

By virtue of working within a copy on write filesystem with provisions for ZFS
style checksums (that is, checksums with the pointers, not the data), we’re
able to use a modern AEAD style construction. We use ChaCha20 and Poly1305. We
use the cyphers directly instead of using the kernel AEAD library (and thus
means there's a bit more in the design that needs auditing).

The current design uses the same key for both ChaCha20 and Poly1305, but my
recent rereading of the Poly1305-AES paper seems to imply that the Poly1305 key
shouldn't be used for anything else. Guidance from actual cryptographers would
be appreciated here; the ChaCha20/Poly1305 AEAD RFC appears to be silent on the
matter.

Note that ChaCha20 is a stream cypher. This means that it’s critical that we use
a cryptographic MAC (which would be highly desirable anyways), and also avoiding
nonce reuse is critical. Getting nonces right is where most of the trickiness is
involved in bcachefs’s encryption.

The current algorithm choices are not hard coded. Bcachefs already has
selectable checksum types, and every individual data and metadata write has a
field that describes the checksum algorithm that was used. On disk, encrypted
data is represented as a new checksum type - so we now have [none, crc32c,
crc64, chacha20/poly1305] as possible methods for data to be
checksummed/encrypted. If in the future we add new encryption algorithms, users
will be able to switch to the new algorithm on existing encrypted filesystems;
new data will be written with the new algorithm and old data will be read with
the old algorithm until it is rewritten.

### Key derivation, master key

Userspace tooling takes the user's passphrase and derives an encryption key with
scrypt. This key is made available to the kernel (via the Linux kernel's keyring
service) prior to mounting the filesystem.

On filesystem mount, the userspace provided key is used to decrypt the master
key, which is stored in the superblock - also with ChaCha20. The master key is
encrypted with an 8 byte header, so that we can tell if the correct key was
supplied.

### Metadata

Except for the superblock, no metadata in bcache/bcachefs is updated in place -
everything is more or less log 

Re: [PATCH 2/2] mm: Add sysfs interface to dump each node's zonelist information

[Anshuman Khandual]

On 09/01/2016 02:42 AM, Andrew Morton wrote:
> On Wed, 31 Aug 2016 08:55:50 +0530 Anshuman Khandual 
>  wrote:
> 
>> Each individual node in the system has a ZONELIST_FALLBACK zonelist
>> and a ZONELIST_NOFALLBACK zonelist. These zonelists decide fallback
>> order of zones during memory allocations. Sometimes it helps to dump
>> these zonelists to see the priority order of various zones in them.
>> This change just adds a sysfs interface for doing the same.
>>
>> Example zonelist information from a KVM guest.
>>
>> [NODE (0)]
>> ZONELIST_FALLBACK
>> (0) (node 0) (zone DMA c140c000)
>> (1) (node 1) (zone DMA c001)
>> (2) (node 2) (zone DMA c002)
>> (3) (node 3) (zone DMA c003)
>> ZONELIST_NOFALLBACK
>> (0) (node 0) (zone DMA c140c000)
>> [NODE (1)]
>> ZONELIST_FALLBACK
>> (0) (node 1) (zone DMA c001)
>> (1) (node 2) (zone DMA c002)
>> (2) (node 3) (zone DMA c003)
>> (3) (node 0) (zone DMA c140c000)
>> ZONELIST_NOFALLBACK
>> (0) (node 1) (zone DMA c001)
>> [NODE (2)]
>> ZONELIST_FALLBACK
>> (0) (node 2) (zone DMA c002)
>> (1) (node 3) (zone DMA c003)
>> (2) (node 0) (zone DMA c140c000)
>> (3) (node 1) (zone DMA c001)
>> ZONELIST_NOFALLBACK
>> (0) (node 2) (zone DMA c002)
>> [NODE (3)]
>> ZONELIST_FALLBACK
>> (0) (node 3) (zone DMA c003)
>> (1) (node 0) (zone DMA c140c000)
>> (2) (node 1) (zone DMA c001)
>> (3) (node 2) (zone DMA c002)
>> ZONELIST_NOFALLBACK
>> (0) (node 3) (zone DMA c003)
> 
> Can you please sell this a bit better?  Why does it "sometimes help"?
> Why does the benefit of this patch to our users justify the overhead
> and cost?

On platforms which support memory hotplug into previously non existing
(at boot) zones, this interface helps in visualizing which zonelists
of the system, the new hot added memory ends up in. POWER is such a
platform where all the memory detected during boot time remains with
ZONE_DMA for good but then hot plug process can actually get new memory
into ZONE_MOVABLE. So having a way to get the snapshot of the zonelists
on the system after memory or node hot[un]plug is a good thing, IMHO.

> 
> Please document the full path to the sysfs file(s) within the changelog.

Sure, will do.

> 
> Please find somewhere in Documentation/ to document the new interface.
> 

Sure, will create this following file describing the interface.

Documentation/ABI/testing/sysfs-system-zone-details

Re: [PATCH 2/2] mm: Add sysfs interface to dump each node's zonelist information

[Anshuman Khandual]

On 09/01/2016 02:42 AM, Andrew Morton wrote:
> On Wed, 31 Aug 2016 08:55:50 +0530 Anshuman Khandual 
>  wrote:
> 
>> Each individual node in the system has a ZONELIST_FALLBACK zonelist
>> and a ZONELIST_NOFALLBACK zonelist. These zonelists decide fallback
>> order of zones during memory allocations. Sometimes it helps to dump
>> these zonelists to see the priority order of various zones in them.
>> This change just adds a sysfs interface for doing the same.
>>
>> Example zonelist information from a KVM guest.
>>
>> [NODE (0)]
>> ZONELIST_FALLBACK
>> (0) (node 0) (zone DMA c140c000)
>> (1) (node 1) (zone DMA c001)
>> (2) (node 2) (zone DMA c002)
>> (3) (node 3) (zone DMA c003)
>> ZONELIST_NOFALLBACK
>> (0) (node 0) (zone DMA c140c000)
>> [NODE (1)]
>> ZONELIST_FALLBACK
>> (0) (node 1) (zone DMA c001)
>> (1) (node 2) (zone DMA c002)
>> (2) (node 3) (zone DMA c003)
>> (3) (node 0) (zone DMA c140c000)
>> ZONELIST_NOFALLBACK
>> (0) (node 1) (zone DMA c001)
>> [NODE (2)]
>> ZONELIST_FALLBACK
>> (0) (node 2) (zone DMA c002)
>> (1) (node 3) (zone DMA c003)
>> (2) (node 0) (zone DMA c140c000)
>> (3) (node 1) (zone DMA c001)
>> ZONELIST_NOFALLBACK
>> (0) (node 2) (zone DMA c002)
>> [NODE (3)]
>> ZONELIST_FALLBACK
>> (0) (node 3) (zone DMA c003)
>> (1) (node 0) (zone DMA c140c000)
>> (2) (node 1) (zone DMA c001)
>> (3) (node 2) (zone DMA c002)
>> ZONELIST_NOFALLBACK
>> (0) (node 3) (zone DMA c003)
> 
> Can you please sell this a bit better?  Why does it "sometimes help"?
> Why does the benefit of this patch to our users justify the overhead
> and cost?

On platforms which support memory hotplug into previously non existing
(at boot) zones, this interface helps in visualizing which zonelists
of the system, the new hot added memory ends up in. POWER is such a
platform where all the memory detected during boot time remains with
ZONE_DMA for good but then hot plug process can actually get new memory
into ZONE_MOVABLE. So having a way to get the snapshot of the zonelists
on the system after memory or node hot[un]plug is a good thing, IMHO.

> 
> Please document the full path to the sysfs file(s) within the changelog.

Sure, will do.

> 
> Please find somewhere in Documentation/ to document the new interface.
> 

Sure, will create this following file describing the interface.

Documentation/ABI/testing/sysfs-system-zone-details

Re: [PATCH 1/2] mm: Move definition of 'zone_names' array into mmzone.h

[Anshuman Khandual]

On 09/01/2016 02:40 AM, Andrew Morton wrote:
> On Wed, 31 Aug 2016 08:55:49 +0530 Anshuman Khandual 
>  wrote:
> 
>> zone_names[] is used to identify any zone given it's index which
>> can be used in many other places. So moving the definition into
>> include/linux/mmzone.h for broader access.
>>
>> ...
>>
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -341,6 +341,23 @@ enum zone_type {
>>  
>>  };
>>  
>> +static char * const zone_names[__MAX_NR_ZONES] = {
>> +#ifdef CONFIG_ZONE_DMA
>> + "DMA",
>> +#endif
>> +#ifdef CONFIG_ZONE_DMA32
>> + "DMA32",
>> +#endif
>> + "Normal",
>> +#ifdef CONFIG_HIGHMEM
>> + "HighMem",
>> +#endif
>> + "Movable",
>> +#ifdef CONFIG_ZONE_DEVICE
>> + "Device",
>> +#endif
>> +};
>> +
>>  #ifndef __GENERATING_BOUNDS_H
>>  
>>  struct zone {
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 3fbe73a..8e2261c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -207,23 +207,6 @@ int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1] = {
>>  
>>  EXPORT_SYMBOL(totalram_pages);
>>  
>> -static char * const zone_names[MAX_NR_ZONES] = {
>> -#ifdef CONFIG_ZONE_DMA
>> - "DMA",
>> -#endif
>> -#ifdef CONFIG_ZONE_DMA32
>> - "DMA32",
>> -#endif
>> - "Normal",
>> -#ifdef CONFIG_HIGHMEM
>> - "HighMem",
>> -#endif
>> - "Movable",
>> -#ifdef CONFIG_ZONE_DEVICE
>> - "Device",
>> -#endif
>> -};
>> -
>>  char * const migratetype_names[MIGRATE_TYPES] = {
>>  "Unmovable",
>>  "Movable",
> 
> This is worrisome.  On some (ancient) compilers, this will produce a
> copy of that array into each compilation unit which includes mmzone.h.
> 
> On smarter compilers, it will produce a copy of the array in each
> compilation unit which *uses* zone_names[].
> 
> On even smarter compilers (and linkers!), only one copy of zone_names[]
> will exist in vmlinux.
> 
> I don't know if gcc is an "even smarter compiler" and I didn't check,
> and I didn't check which gcc versions are even smarter.  I'd rather not
> have to ;) It is risky.
> 
> So, let's just make it non-static and add a declaration into mmzone.h,
> please.
> 

I understand your concern, will change it.

Re: [PATCH 1/2] mm: Move definition of 'zone_names' array into mmzone.h

[Anshuman Khandual]

On 09/01/2016 02:40 AM, Andrew Morton wrote:
> On Wed, 31 Aug 2016 08:55:49 +0530 Anshuman Khandual 
>  wrote:
> 
>> zone_names[] is used to identify any zone given it's index which
>> can be used in many other places. So moving the definition into
>> include/linux/mmzone.h for broader access.
>>
>> ...
>>
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -341,6 +341,23 @@ enum zone_type {
>>  
>>  };
>>  
>> +static char * const zone_names[__MAX_NR_ZONES] = {
>> +#ifdef CONFIG_ZONE_DMA
>> + "DMA",
>> +#endif
>> +#ifdef CONFIG_ZONE_DMA32
>> + "DMA32",
>> +#endif
>> + "Normal",
>> +#ifdef CONFIG_HIGHMEM
>> + "HighMem",
>> +#endif
>> + "Movable",
>> +#ifdef CONFIG_ZONE_DEVICE
>> + "Device",
>> +#endif
>> +};
>> +
>>  #ifndef __GENERATING_BOUNDS_H
>>  
>>  struct zone {
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 3fbe73a..8e2261c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -207,23 +207,6 @@ int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1] = {
>>  
>>  EXPORT_SYMBOL(totalram_pages);
>>  
>> -static char * const zone_names[MAX_NR_ZONES] = {
>> -#ifdef CONFIG_ZONE_DMA
>> - "DMA",
>> -#endif
>> -#ifdef CONFIG_ZONE_DMA32
>> - "DMA32",
>> -#endif
>> - "Normal",
>> -#ifdef CONFIG_HIGHMEM
>> - "HighMem",
>> -#endif
>> - "Movable",
>> -#ifdef CONFIG_ZONE_DEVICE
>> - "Device",
>> -#endif
>> -};
>> -
>>  char * const migratetype_names[MIGRATE_TYPES] = {
>>  "Unmovable",
>>  "Movable",
> 
> This is worrisome.  On some (ancient) compilers, this will produce a
> copy of that array into each compilation unit which includes mmzone.h.
> 
> On smarter compilers, it will produce a copy of the array in each
> compilation unit which *uses* zone_names[].
> 
> On even smarter compilers (and linkers!), only one copy of zone_names[]
> will exist in vmlinux.
> 
> I don't know if gcc is an "even smarter compiler" and I didn't check,
> and I didn't check which gcc versions are even smarter.  I'd rather not
> have to ;) It is risky.
> 
> So, let's just make it non-static and add a declaration into mmzone.h,
> please.
> 

I understand your concern, will change it.

[PATCH v2 2/4] mmc: dw_mmc: avoid race condition of cpu and IDMAC

[Shawn Lin]

We could see an obvious race condition by test that
the former write operation by IDMAC aiming to clear
OWN bit reach right after the later configuration of
the same desc, which makes the IDMAC be in SUSPEND
state as the OWN bit was cleared by the asynchronous
write operation of IDMAC. The bug can be very easy
reproduced on RK3288 or similar when we reduce the
running rate of system buses and keep the CPU running
faster. So as two separate masters, IDMAC and cpu
write the same descriptor stored on the same address,
and this should be protected by adding check of OWN
bit before preparing new descriptors.

Signed-off-by: Shawn Lin 

---

Changes in v2:
- fallback to PIO mode if failing to wait for OWN bit
- cleanup polluted desc chain as the own bit error could
  occur on any place of the chain, so we need to clr the desc
  configured before that one. Use the exist function to reinit
  the desc chain. As this issue is really rare, so after applied,
  the fio/iozone stress test didn't show up performance regression.

 drivers/mmc/host/dw_mmc.c | 208 --
 1 file changed, 129 insertions(+), 79 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 782b303..daa1c52 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -467,12 +467,87 @@ static void dw_mci_dmac_complete_dma(void *arg)
}
 }
 
-static inline void dw_mci_prepare_desc64(struct dw_mci *host,
+static int dw_mci_idmac_init(struct dw_mci *host)
+{
+   int i;
+
+   if (host->dma_64bit_address == 1) {
+   struct idmac_desc_64addr *p;
+   /* Number of descriptors in the ring buffer */
+   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc_64addr);
+
+   /* Forward link the descriptor list */
+   for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
+   i++, p++) {
+   p->des6 = (host->sg_dma +
+   (sizeof(struct idmac_desc_64addr) *
+   (i + 1))) & 0x;
+
+   p->des7 = (u64)(host->sg_dma +
+   (sizeof(struct idmac_desc_64addr) *
+   (i + 1))) >> 32;
+   /* Initialize reserved and buffer size fields to "0" */
+   p->des1 = 0;
+   p->des2 = 0;
+   p->des3 = 0;
+   }
+
+   /* Set the last descriptor as the end-of-ring descriptor */
+   p->des6 = host->sg_dma & 0x;
+   p->des7 = (u64)host->sg_dma >> 32;
+   p->des0 = IDMAC_DES0_ER;
+
+   } else {
+   struct idmac_desc *p;
+   /* Number of descriptors in the ring buffer */
+   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
+
+   /* Forward link the descriptor list */
+   for (i = 0, p = host->sg_cpu;
+i < host->ring_size - 1;
+i++, p++) {
+   p->des3 = cpu_to_le32(host->sg_dma +
+   (sizeof(struct idmac_desc) * (i + 1)));
+   p->des1 = 0;
+   }
+
+   /* Set the last descriptor as the end-of-ring descriptor */
+   p->des3 = cpu_to_le32(host->sg_dma);
+   p->des0 = cpu_to_le32(IDMAC_DES0_ER);
+   }
+
+   dw_mci_idmac_reset(host);
+
+   if (host->dma_64bit_address == 1) {
+   /* Mask out interrupts - get Tx & Rx complete only */
+   mci_writel(host, IDSTS64, IDMAC_INT_CLR);
+   mci_writel(host, IDINTEN64, SDMMC_IDMAC_INT_NI |
+   SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
+
+   /* Set the descriptor base address */
+   mci_writel(host, DBADDRL, host->sg_dma & 0x);
+   mci_writel(host, DBADDRU, (u64)host->sg_dma >> 32);
+
+   } else {
+   /* Mask out interrupts - get Tx & Rx complete only */
+   mci_writel(host, IDSTS, IDMAC_INT_CLR);
+   mci_writel(host, IDINTEN, SDMMC_IDMAC_INT_NI |
+   SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
+
+   /* Set the descriptor base address */
+   mci_writel(host, DBADDR, host->sg_dma);
+   }
+
+   return 0;
+}
+
+static inline int dw_mci_prepare_desc64(struct dw_mci *host,
 struct mmc_data *data,
 unsigned int sg_len)
 {
unsigned int desc_len;
struct idmac_desc_64addr *desc_first, *desc_last, *desc;
+   unsigned long timeout;
int i;
 
desc_first = desc_last = desc = host->sg_cpu;
@@ -489,6 +564,19 @@ 

[PATCH v2 2/4] mmc: dw_mmc: avoid race condition of cpu and IDMAC

[Shawn Lin]

We could see an obvious race condition by test that
the former write operation by IDMAC aiming to clear
OWN bit reach right after the later configuration of
the same desc, which makes the IDMAC be in SUSPEND
state as the OWN bit was cleared by the asynchronous
write operation of IDMAC. The bug can be very easy
reproduced on RK3288 or similar when we reduce the
running rate of system buses and keep the CPU running
faster. So as two separate masters, IDMAC and cpu
write the same descriptor stored on the same address,
and this should be protected by adding check of OWN
bit before preparing new descriptors.

Signed-off-by: Shawn Lin 

---

Changes in v2:
- fallback to PIO mode if failing to wait for OWN bit
- cleanup polluted desc chain as the own bit error could
  occur on any place of the chain, so we need to clr the desc
  configured before that one. Use the exist function to reinit
  the desc chain. As this issue is really rare, so after applied,
  the fio/iozone stress test didn't show up performance regression.

 drivers/mmc/host/dw_mmc.c | 208 --
 1 file changed, 129 insertions(+), 79 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 782b303..daa1c52 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -467,12 +467,87 @@ static void dw_mci_dmac_complete_dma(void *arg)
}
 }
 
-static inline void dw_mci_prepare_desc64(struct dw_mci *host,
+static int dw_mci_idmac_init(struct dw_mci *host)
+{
+   int i;
+
+   if (host->dma_64bit_address == 1) {
+   struct idmac_desc_64addr *p;
+   /* Number of descriptors in the ring buffer */
+   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc_64addr);
+
+   /* Forward link the descriptor list */
+   for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
+   i++, p++) {
+   p->des6 = (host->sg_dma +
+   (sizeof(struct idmac_desc_64addr) *
+   (i + 1))) & 0x;
+
+   p->des7 = (u64)(host->sg_dma +
+   (sizeof(struct idmac_desc_64addr) *
+   (i + 1))) >> 32;
+   /* Initialize reserved and buffer size fields to "0" */
+   p->des1 = 0;
+   p->des2 = 0;
+   p->des3 = 0;
+   }
+
+   /* Set the last descriptor as the end-of-ring descriptor */
+   p->des6 = host->sg_dma & 0x;
+   p->des7 = (u64)host->sg_dma >> 32;
+   p->des0 = IDMAC_DES0_ER;
+
+   } else {
+   struct idmac_desc *p;
+   /* Number of descriptors in the ring buffer */
+   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
+
+   /* Forward link the descriptor list */
+   for (i = 0, p = host->sg_cpu;
+i < host->ring_size - 1;
+i++, p++) {
+   p->des3 = cpu_to_le32(host->sg_dma +
+   (sizeof(struct idmac_desc) * (i + 1)));
+   p->des1 = 0;
+   }
+
+   /* Set the last descriptor as the end-of-ring descriptor */
+   p->des3 = cpu_to_le32(host->sg_dma);
+   p->des0 = cpu_to_le32(IDMAC_DES0_ER);
+   }
+
+   dw_mci_idmac_reset(host);
+
+   if (host->dma_64bit_address == 1) {
+   /* Mask out interrupts - get Tx & Rx complete only */
+   mci_writel(host, IDSTS64, IDMAC_INT_CLR);
+   mci_writel(host, IDINTEN64, SDMMC_IDMAC_INT_NI |
+   SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
+
+   /* Set the descriptor base address */
+   mci_writel(host, DBADDRL, host->sg_dma & 0x);
+   mci_writel(host, DBADDRU, (u64)host->sg_dma >> 32);
+
+   } else {
+   /* Mask out interrupts - get Tx & Rx complete only */
+   mci_writel(host, IDSTS, IDMAC_INT_CLR);
+   mci_writel(host, IDINTEN, SDMMC_IDMAC_INT_NI |
+   SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
+
+   /* Set the descriptor base address */
+   mci_writel(host, DBADDR, host->sg_dma);
+   }
+
+   return 0;
+}
+
+static inline int dw_mci_prepare_desc64(struct dw_mci *host,
 struct mmc_data *data,
 unsigned int sg_len)
 {
unsigned int desc_len;
struct idmac_desc_64addr *desc_first, *desc_last, *desc;
+   unsigned long timeout;
int i;
 
desc_first = desc_last = desc = host->sg_cpu;
@@ -489,6 +564,19 @@ static inline void 

[PATCH v2 4/4] mmc: dw_mmc: use macro to define ring buffer size

[Shawn Lin]

It's very prone to make mistake as we might forget
to replace all PAGE_SIZEs with new values if we try
to modify the ring buffer size for whatever reasons.
Let's use a macro to define it.

Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 833b6ad..6e5926b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -61,6 +61,8 @@
 SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
 SDMMC_IDMAC_INT_TI)
 
+#define DESC_RING_BUF_SZ   PAGE_SIZE
+
 struct idmac_desc_64addr {
u32 des0;   /* Control Descriptor */
 
@@ -474,7 +476,8 @@ static int dw_mci_idmac_init(struct dw_mci *host)
if (host->dma_64bit_address == 1) {
struct idmac_desc_64addr *p;
/* Number of descriptors in the ring buffer */
-   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc_64addr);
+   host->ring_size =
+   DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
 
/* Forward link the descriptor list */
for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
@@ -500,7 +503,8 @@ static int dw_mci_idmac_init(struct dw_mci *host)
} else {
struct idmac_desc *p;
/* Number of descriptors in the ring buffer */
-   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
+   host->ring_size =
+   DESC_RING_BUF_SZ / sizeof(struct idmac_desc);
 
/* Forward link the descriptor list */
for (i = 0, p = host->sg_cpu;
@@ -609,7 +613,7 @@ static inline int dw_mci_prepare_desc64(struct dw_mci *host,
 err_own_bit:
/* restore the descriptor chain as it's polluted */
dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n");
-   memset(host->sg_cpu, 0, PAGE_SIZE);
+   memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
dw_mci_idmac_init(host);
return -EINVAL;
 }
@@ -685,7 +689,7 @@ static inline int dw_mci_prepare_desc32(struct dw_mci *host,
 err_own_bit:
/* restore the descriptor chain as it's polluted */
dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n");
-   memset(host->sg_cpu, 0, PAGE_SIZE);
+   memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
dw_mci_idmac_init(host);
return -EINVAL;
 }
@@ -2750,7 +2754,8 @@ static void dw_mci_init_dma(struct dw_mci *host)
}
 
/* Alloc memory for sg translation */
-   host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
+   host->sg_cpu = dmam_alloc_coherent(host->dev,
+  DESC_RING_BUF_SZ,
   >sg_dma, GFP_KERNEL);
if (!host->sg_cpu) {
dev_err(host->dev,
-- 
2.3.7

[PATCH v2 4/4] mmc: dw_mmc: use macro to define ring buffer size

[Shawn Lin]

It's very prone to make mistake as we might forget
to replace all PAGE_SIZEs with new values if we try
to modify the ring buffer size for whatever reasons.
Let's use a macro to define it.

Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 833b6ad..6e5926b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -61,6 +61,8 @@
 SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
 SDMMC_IDMAC_INT_TI)
 
+#define DESC_RING_BUF_SZ   PAGE_SIZE
+
 struct idmac_desc_64addr {
u32 des0;   /* Control Descriptor */
 
@@ -474,7 +476,8 @@ static int dw_mci_idmac_init(struct dw_mci *host)
if (host->dma_64bit_address == 1) {
struct idmac_desc_64addr *p;
/* Number of descriptors in the ring buffer */
-   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc_64addr);
+   host->ring_size =
+   DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
 
/* Forward link the descriptor list */
for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
@@ -500,7 +503,8 @@ static int dw_mci_idmac_init(struct dw_mci *host)
} else {
struct idmac_desc *p;
/* Number of descriptors in the ring buffer */
-   host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
+   host->ring_size =
+   DESC_RING_BUF_SZ / sizeof(struct idmac_desc);
 
/* Forward link the descriptor list */
for (i = 0, p = host->sg_cpu;
@@ -609,7 +613,7 @@ static inline int dw_mci_prepare_desc64(struct dw_mci *host,
 err_own_bit:
/* restore the descriptor chain as it's polluted */
dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n");
-   memset(host->sg_cpu, 0, PAGE_SIZE);
+   memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
dw_mci_idmac_init(host);
return -EINVAL;
 }
@@ -685,7 +689,7 @@ static inline int dw_mci_prepare_desc32(struct dw_mci *host,
 err_own_bit:
/* restore the descriptor chain as it's polluted */
dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n");
-   memset(host->sg_cpu, 0, PAGE_SIZE);
+   memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
dw_mci_idmac_init(host);
return -EINVAL;
 }
@@ -2750,7 +2754,8 @@ static void dw_mci_init_dma(struct dw_mci *host)
}
 
/* Alloc memory for sg translation */
-   host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
+   host->sg_cpu = dmam_alloc_coherent(host->dev,
+  DESC_RING_BUF_SZ,
   >sg_dma, GFP_KERNEL);
if (!host->sg_cpu) {
dev_err(host->dev,
-- 
2.3.7

[PATCH v2 3/4] mmc: dw_mmc: fix misleading error print if failing to do DMA transfer

[Shawn Lin]

The original log didn't figure out that we could still
finish this transfer by PIO mode even if failing to use
DMA. And it should be kept for debug level instead of
error one.

Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index daa1c52..833b6ad 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1057,8 +1057,10 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, 
struct mmc_data *data)
spin_unlock_irqrestore(>irq_lock, irqflags);
 
if (host->dma_ops->start(host, sg_len)) {
-   /* We can't do DMA */
-   dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
+   /* We can't do DMA, try PIO for this one */
+   dev_dbg(host->dev,
+   "%s: fall back to PIO mode for current transfer\n",
+   __func__);
return -ENODEV;
}
 
-- 
2.3.7

[PATCH v2 1/4] mmc: dw_mmc: split out preparation of desc for IDMAC32 and IDMAC64

[Shawn Lin]

We intend to add more check for descriptors when
preparing desc. Let's spilt out the separate body
to make the dw_mci_translate_sglist not so lengthy.
After spliting out these two functions, we could
remove dw_mci_translate_sglist and call both of them
when staring idmac.

Signed-off-by: Shawn Lin 

---

Changes in v2:
- remove dw_mci_translate_sglist

 drivers/mmc/host/dw_mmc.c | 149 --
 1 file changed, 79 insertions(+), 70 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 22dacae..782b303 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -467,112 +467,121 @@ static void dw_mci_dmac_complete_dma(void *arg)
}
 }
 
-static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
-   unsigned int sg_len)
+static inline void dw_mci_prepare_desc64(struct dw_mci *host,
+struct mmc_data *data,
+unsigned int sg_len)
 {
unsigned int desc_len;
+   struct idmac_desc_64addr *desc_first, *desc_last, *desc;
int i;
 
-   if (host->dma_64bit_address == 1) {
-   struct idmac_desc_64addr *desc_first, *desc_last, *desc;
-
-   desc_first = desc_last = desc = host->sg_cpu;
+   desc_first = desc_last = desc = host->sg_cpu;
 
-   for (i = 0; i < sg_len; i++) {
-   unsigned int length = sg_dma_len(>sg[i]);
+   for (i = 0; i < sg_len; i++) {
+   unsigned int length = sg_dma_len(>sg[i]);
 
-   u64 mem_addr = sg_dma_address(>sg[i]);
+   u64 mem_addr = sg_dma_address(>sg[i]);
 
-   for ( ; length ; desc++) {
-   desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
-  length : DW_MCI_DESC_DATA_LENGTH;
+   for ( ; length ; desc++) {
+   desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+  length : DW_MCI_DESC_DATA_LENGTH;
 
-   length -= desc_len;
+   length -= desc_len;
 
-   /*
-* Set the OWN bit and disable interrupts
-* for this descriptor
-*/
-   desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
-   IDMAC_DES0_CH;
+   /*
+* Set the OWN bit and disable interrupts
+* for this descriptor
+*/
+   desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
+   IDMAC_DES0_CH;
 
-   /* Buffer length */
-   IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
+   /* Buffer length */
+   IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
 
-   /* Physical address to DMA to/from */
-   desc->des4 = mem_addr & 0x;
-   desc->des5 = mem_addr >> 32;
+   /* Physical address to DMA to/from */
+   desc->des4 = mem_addr & 0x;
+   desc->des5 = mem_addr >> 32;
 
-   /* Update physical address for the next desc */
-   mem_addr += desc_len;
+   /* Update physical address for the next desc */
+   mem_addr += desc_len;
 
-   /* Save pointer to the last descriptor */
-   desc_last = desc;
-   }
+   /* Save pointer to the last descriptor */
+   desc_last = desc;
}
+   }
 
-   /* Set first descriptor */
-   desc_first->des0 |= IDMAC_DES0_FD;
+   /* Set first descriptor */
+   desc_first->des0 |= IDMAC_DES0_FD;
 
-   /* Set last descriptor */
-   desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
-   desc_last->des0 |= IDMAC_DES0_LD;
+   /* Set last descriptor */
+   desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
+   desc_last->des0 |= IDMAC_DES0_LD;
+}
 
-   } else {
-   struct idmac_desc *desc_first, *desc_last, *desc;
 
-   desc_first = desc_last = desc = host->sg_cpu;
+static inline void dw_mci_prepare_desc32(struct dw_mci *host,
+struct mmc_data *data,
+unsigned int sg_len)
+{
+   unsigned int desc_len;
+   struct idmac_desc *desc_first, *desc_last, *desc;
+

[PATCH v2 3/4] mmc: dw_mmc: fix misleading error print if failing to do DMA transfer

[Shawn Lin]

The original log didn't figure out that we could still
finish this transfer by PIO mode even if failing to use
DMA. And it should be kept for debug level instead of
error one.

Signed-off-by: Shawn Lin 
---

Changes in v2: None

 drivers/mmc/host/dw_mmc.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index daa1c52..833b6ad 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1057,8 +1057,10 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, 
struct mmc_data *data)
spin_unlock_irqrestore(>irq_lock, irqflags);
 
if (host->dma_ops->start(host, sg_len)) {
-   /* We can't do DMA */
-   dev_err(host->dev, "%s: failed to start DMA.\n", __func__);
+   /* We can't do DMA, try PIO for this one */
+   dev_dbg(host->dev,
+   "%s: fall back to PIO mode for current transfer\n",
+   __func__);
return -ENODEV;
}
 
-- 
2.3.7

[PATCH v2 1/4] mmc: dw_mmc: split out preparation of desc for IDMAC32 and IDMAC64

[Shawn Lin]

We intend to add more check for descriptors when
preparing desc. Let's spilt out the separate body
to make the dw_mci_translate_sglist not so lengthy.
After spliting out these two functions, we could
remove dw_mci_translate_sglist and call both of them
when staring idmac.

Signed-off-by: Shawn Lin 

---

Changes in v2:
- remove dw_mci_translate_sglist

 drivers/mmc/host/dw_mmc.c | 149 --
 1 file changed, 79 insertions(+), 70 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 22dacae..782b303 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -467,112 +467,121 @@ static void dw_mci_dmac_complete_dma(void *arg)
}
 }
 
-static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
-   unsigned int sg_len)
+static inline void dw_mci_prepare_desc64(struct dw_mci *host,
+struct mmc_data *data,
+unsigned int sg_len)
 {
unsigned int desc_len;
+   struct idmac_desc_64addr *desc_first, *desc_last, *desc;
int i;
 
-   if (host->dma_64bit_address == 1) {
-   struct idmac_desc_64addr *desc_first, *desc_last, *desc;
-
-   desc_first = desc_last = desc = host->sg_cpu;
+   desc_first = desc_last = desc = host->sg_cpu;
 
-   for (i = 0; i < sg_len; i++) {
-   unsigned int length = sg_dma_len(>sg[i]);
+   for (i = 0; i < sg_len; i++) {
+   unsigned int length = sg_dma_len(>sg[i]);
 
-   u64 mem_addr = sg_dma_address(>sg[i]);
+   u64 mem_addr = sg_dma_address(>sg[i]);
 
-   for ( ; length ; desc++) {
-   desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
-  length : DW_MCI_DESC_DATA_LENGTH;
+   for ( ; length ; desc++) {
+   desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ?
+  length : DW_MCI_DESC_DATA_LENGTH;
 
-   length -= desc_len;
+   length -= desc_len;
 
-   /*
-* Set the OWN bit and disable interrupts
-* for this descriptor
-*/
-   desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
-   IDMAC_DES0_CH;
+   /*
+* Set the OWN bit and disable interrupts
+* for this descriptor
+*/
+   desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
+   IDMAC_DES0_CH;
 
-   /* Buffer length */
-   IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
+   /* Buffer length */
+   IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len);
 
-   /* Physical address to DMA to/from */
-   desc->des4 = mem_addr & 0x;
-   desc->des5 = mem_addr >> 32;
+   /* Physical address to DMA to/from */
+   desc->des4 = mem_addr & 0x;
+   desc->des5 = mem_addr >> 32;
 
-   /* Update physical address for the next desc */
-   mem_addr += desc_len;
+   /* Update physical address for the next desc */
+   mem_addr += desc_len;
 
-   /* Save pointer to the last descriptor */
-   desc_last = desc;
-   }
+   /* Save pointer to the last descriptor */
+   desc_last = desc;
}
+   }
 
-   /* Set first descriptor */
-   desc_first->des0 |= IDMAC_DES0_FD;
+   /* Set first descriptor */
+   desc_first->des0 |= IDMAC_DES0_FD;
 
-   /* Set last descriptor */
-   desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
-   desc_last->des0 |= IDMAC_DES0_LD;
+   /* Set last descriptor */
+   desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
+   desc_last->des0 |= IDMAC_DES0_LD;
+}
 
-   } else {
-   struct idmac_desc *desc_first, *desc_last, *desc;
 
-   desc_first = desc_last = desc = host->sg_cpu;
+static inline void dw_mci_prepare_desc32(struct dw_mci *host,
+struct mmc_data *data,
+unsigned int sg_len)
+{
+   unsigned int desc_len;
+   struct idmac_desc *desc_first, *desc_last, *desc;
+   int i;
 
- 

Re: [PATCH v5 2/4] ARM: dts: rockchip: update compatible strings for Rockchip efuse

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  arch/arm/boot/dts/rk3066a.dtsi | 2 +-
>  arch/arm/boot/dts/rk3188.dtsi  | 2 +-
>  arch/arm/boot/dts/rk3288.dtsi  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 2/4] ARM: dts: rockchip: update compatible strings for Rockchip efuse

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  arch/arm/boot/dts/rk3066a.dtsi | 2 +-
>  arch/arm/boot/dts/rk3188.dtsi  | 2 +-
>  arch/arm/boot/dts/rk3288.dtsi  | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 1/4] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
> fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
> programmable electrical fuses with random access interface.
>
> Add different device tree compatible string for different SoCs to be able
> to differentiate between the two. The old binding is of course preserved,
> though deprecated.
>
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 1/4] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
> fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
> programmable electrical fuses with random access interface.
>
> Add different device tree compatible string for different SoCs to be able
> to differentiate between the two. The old binding is of course preserved,
> though deprecated.
>
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 4/4] nvmem: rockchip-efuse: add rk3399-efuse support

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> 1) the efuse timing of rk3399 is different from earlier SoCs.
> 2) rk3399-efuse is organized as 32bits by 32 one-time programmable
> electrical fuses, the efuse of earlier SoCs is organized as 32bits
> by 8 one-time programmable electrical fuses with random access interface.
>
> This patch adds a new read function for rk3399-efuse.
>
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  drivers/nvmem/rockchip-efuse.c | 133 
> +++--
>  1 file changed, 114 insertions(+), 19 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 4/4] nvmem: rockchip-efuse: add rk3399-efuse support

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> 1) the efuse timing of rk3399 is different from earlier SoCs.
> 2) rk3399-efuse is organized as 32bits by 32 one-time programmable
> electrical fuses, the efuse of earlier SoCs is organized as 32bits
> by 8 one-time programmable electrical fuses with random access interface.
>
> This patch adds a new read function for rk3399-efuse.
>
> Signed-off-by: Finley Xiao 
> Reviewed-by: Heiko Stuebner 
> ---
>  drivers/nvmem/rockchip-efuse.c | 133 
> +++--
>  1 file changed, 114 insertions(+), 19 deletions(-)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 3/4] arm64: dts: rockchip: add efuse0 device node for rk3399

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Add a efuse0 node in the device tree for the ARM64 rk3399 SoC.
>
> Signed-off-by: Finley Xiao 
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 29 +
>  1 file changed, 29 insertions(+)

Reviewed-by: Douglas Anderson 

Re: [PATCH v5 3/4] arm64: dts: rockchip: add efuse0 device node for rk3399

[Doug Anderson]

Hi,

On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiao  wrote:
> Add a efuse0 node in the device tree for the ARM64 rk3399 SoC.
>
> Signed-off-by: Finley Xiao 
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 29 +
>  1 file changed, 29 insertions(+)

Reviewed-by: Douglas Anderson 

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Rafael

在 2016/9/2 7:38, Rafael J. Wysocki 写道:

On Thursday, September 01, 2016 11:23:42 AM Dongdong Liu wrote:


在 2016/9/1 6:56, Rafael J. Wysocki 写道:

On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:

Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_acpi_init() to get RC config resource
with hardcoded range address and setup ecam mapping.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu 
Signed-off-by: Gabriele Paoloni 


Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
or is there anything more to it?



Hi Rafael, thanks for replying.

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

In our case we cannot use the standard MCFG object to pass the RC itself config 
space addresses.
The more discuss information can be found:
https://lkml.org/lkml/2016/2/22/1087
[...]
I have looked into this and in our case we cannot use the
standard MCFG object to pass the RC config space addresses.

The reason is that in our HW we have the config base addresses of the
root complex ports that are less than 0x10 byte distant one from
the other as we only map the first 0x1 bytes.
Now the MCFG acpi framework always fix the MCFG resource size to 0x10
for each bus; therefore if we pass our RC addresses through MCFG we end
up with a resource conflict.
To give you a practical example we are in a situation where we have:

port0: [0xb008 - 0xb008 + 0x1]
port1: [0xb009 - 0xb009 + 0x1]
port2: [0xb00A - 0xb00A + 0x1]
port3: [0xb00B - 0xb00B + 0x1]
So if we pass the base addresses through MCFG the resources
will overlap as MCFG will consider 0x10 size for each base
address of the root complex (only the RC bus uses that address)
So far I do not see many option other than using _DSD to pass
these RC config base addresses.


It still is not entirely clear to me what the "ACPI support" is here.

Do you read any configuration information from the ACPI tables or similar?

If so, where is the format of it documented?



Since Our host bridge is non ECAM only for the RC bus config space,for any 
other bus underneath the root bus we support ECAM access,
we need to override these accessors prior to PCI buses enumeration.

As below is MCFG table configuration.
0x2200400~0x220040f (bus 0x40) addresses are wasted, because Our host 
bridge is non ECAM only for the RC.
We use RC base address(0xb008)+offset to access our RC config space.
0x2200410~0x22007ff addresses are used to aceess EP config space (bus 
0x41~0x7f).This support ECAM access.

MCFG Table
...
{
  0x220,  //Base Address
  0x0001, //Segment Group Number
  0x40,   //Start Bus Number
  0x7f,   //End Bus Number
  0x, //Reserved
},
...

DSDT Table
//PCIe Root bus
Device (PCI1)
{
Name (_HID, "PNP0A08") // PCI Express Root Bridge
Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
Name(_SEG, 1) // Segment of this Root complex
Name(_BBN, 0x40) // Base Bus Number
Name(_CCA, 1)
Method (_CRS, 0, Serialized) { // Root complex resources
Name (RBUF, ResourceTemplate () {
WordBusNumber ( // Bus numbers assigned to this root
ResourceProducer, MinFixed, MaxFixed, PosDecode,
0,// AddressGranularity
0x40, // AddressMinimum - Minimum Bus Number
0x7f, // AddressMaximum - Maximum Bus Number
0,// AddressTranslation - Set to 0
0x40  // RangeLength - Number of Busses
)
...
...
}) // Name(RBUF)
  Return (RBUF)
} // Method(_CRS)
} // Device(PCI1)


As below hisi_pcie_acpi_rd_conf() is our config read implementation.
static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
  int size, u32 *val)
{
struct pci_config_window *cfg = bus->sysdata;
void __iomem *reg_base = cfg->priv;

if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
return PCIBIOS_DEVICE_NOT_FOUND;
/* Access RC config space */
if (bus->number == cfg->busr.start)
return hisi_pcie_common_cfg_read(reg_base, where, size, val);

/* Access EP config space */
return pci_generic_config_read(bus, 

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Rafael

在 2016/9/2 7:38, Rafael J. Wysocki 写道:

On Thursday, September 01, 2016 11:23:42 AM Dongdong Liu wrote:


在 2016/9/1 6:56, Rafael J. Wysocki 写道:

On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:

Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_acpi_init() to get RC config resource
with hardcoded range address and setup ecam mapping.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu 
Signed-off-by: Gabriele Paoloni 


Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
or is there anything more to it?



Hi Rafael, thanks for replying.

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

In our case we cannot use the standard MCFG object to pass the RC itself config 
space addresses.
The more discuss information can be found:
https://lkml.org/lkml/2016/2/22/1087
[...]
I have looked into this and in our case we cannot use the
standard MCFG object to pass the RC config space addresses.

The reason is that in our HW we have the config base addresses of the
root complex ports that are less than 0x10 byte distant one from
the other as we only map the first 0x1 bytes.
Now the MCFG acpi framework always fix the MCFG resource size to 0x10
for each bus; therefore if we pass our RC addresses through MCFG we end
up with a resource conflict.
To give you a practical example we are in a situation where we have:

port0: [0xb008 - 0xb008 + 0x1]
port1: [0xb009 - 0xb009 + 0x1]
port2: [0xb00A - 0xb00A + 0x1]
port3: [0xb00B - 0xb00B + 0x1]
So if we pass the base addresses through MCFG the resources
will overlap as MCFG will consider 0x10 size for each base
address of the root complex (only the RC bus uses that address)
So far I do not see many option other than using _DSD to pass
these RC config base addresses.


It still is not entirely clear to me what the "ACPI support" is here.

Do you read any configuration information from the ACPI tables or similar?

If so, where is the format of it documented?



Since Our host bridge is non ECAM only for the RC bus config space,for any 
other bus underneath the root bus we support ECAM access,
we need to override these accessors prior to PCI buses enumeration.

As below is MCFG table configuration.
0x2200400~0x220040f (bus 0x40) addresses are wasted, because Our host 
bridge is non ECAM only for the RC.
We use RC base address(0xb008)+offset to access our RC config space.
0x2200410~0x22007ff addresses are used to aceess EP config space (bus 
0x41~0x7f).This support ECAM access.

MCFG Table
...
{
  0x220,  //Base Address
  0x0001, //Segment Group Number
  0x40,   //Start Bus Number
  0x7f,   //End Bus Number
  0x, //Reserved
},
...

DSDT Table
//PCIe Root bus
Device (PCI1)
{
Name (_HID, "PNP0A08") // PCI Express Root Bridge
Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
Name(_SEG, 1) // Segment of this Root complex
Name(_BBN, 0x40) // Base Bus Number
Name(_CCA, 1)
Method (_CRS, 0, Serialized) { // Root complex resources
Name (RBUF, ResourceTemplate () {
WordBusNumber ( // Bus numbers assigned to this root
ResourceProducer, MinFixed, MaxFixed, PosDecode,
0,// AddressGranularity
0x40, // AddressMinimum - Minimum Bus Number
0x7f, // AddressMaximum - Maximum Bus Number
0,// AddressTranslation - Set to 0
0x40  // RangeLength - Number of Busses
)
...
...
}) // Name(RBUF)
  Return (RBUF)
} // Method(_CRS)
} // Device(PCI1)


As below hisi_pcie_acpi_rd_conf() is our config read implementation.
static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
  int size, u32 *val)
{
struct pci_config_window *cfg = bus->sysdata;
void __iomem *reg_base = cfg->priv;

if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
return PCIBIOS_DEVICE_NOT_FOUND;
/* Access RC config space */
if (bus->number == cfg->busr.start)
return hisi_pcie_common_cfg_read(reg_base, where, size, val);

/* Access EP config space */
return pci_generic_config_read(bus, devfn, where, size, val);
}

Thanks
Dongdong

Thanks,

Re: [PATCH v3 18/22] usb: chipidea: msm: Add reset controller for PHY POR bit

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:32PM -0700, Stephen Boyd wrote:
> The MSM chipidea wrapper has two bits that are used to reset the
> first or second phy. Add support for these bits via the reset
> controller framework, so that phy drivers can reset their
> hardware at the right time during initialization.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/Kconfig   |  1 +
>  drivers/usb/chipidea/ci_hdrc_msm.c | 50 
> --
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index 19c20eaa23f2..fc96f5cdcb5c 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -2,6 +2,7 @@ config USB_CHIPIDEA
>   tristate "ChipIdea Highspeed Dual Role Controller"
>   depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && 
> !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
>   select EXTCON
> + select RESET_CONTROLLER
>   help
> Say Y here if your system has a dual role high speed USB
> controller based on ChipIdea silicon IP. It supports:
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c 
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 2489a63d3e75..fe96df7b530c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -31,8 +32,10 @@
>  #define HSPHY_SESS_VLD_CTRL  BIT(25)
>  
>  /* Vendor base starts at 0x200 beyond CI base */
> +#define HS_PHY_CTRL  0x0040
>  #define HS_PHY_SEC_CTRL  0x0078
>  #define HS_PHY_DIG_CLAMP_N   BIT(16)
> +#define HS_PHY_POR_ASSERTBIT(0)
>  
>  struct ci_hdrc_msm {
>   struct platform_device *ci;
> @@ -40,11 +43,43 @@ struct ci_hdrc_msm {
>   struct clk *iface_clk;
>   struct clk *fs_clk;
>   struct ci_hdrc_platform_data pdata;
> + struct reset_controller_dev rcdev;
>   bool secondary_phy;
>   bool hsic;
>   void __iomem *base;
>  };
>  
> +static int
> +ci_hdrc_msm_por_reset(struct reset_controller_dev *r, unsigned long id)
> +{
> + struct ci_hdrc_msm *ci_msm = container_of(r, struct ci_hdrc_msm, rcdev);
> + void __iomem *addr = ci_msm->base;
> + u32 val;
> +
> + if (id)
> + addr += HS_PHY_SEC_CTRL;
> + else
> + addr += HS_PHY_CTRL;
> +
> + val = readl_relaxed(addr);
> + val |= HS_PHY_POR_ASSERT;
> + writel(val, addr);
> + /*
> +  * wait for minimum 10 microseconds as suggested by manual.
> +  * Use a slightly larger value since the exact value didn't
> +  * work 100% of the time.
> +  */
> + udelay(12);
> + val &= ~HS_PHY_POR_ASSERT;
> + writel(val, addr);
> +
> + return 0;
> +}
> +
> +static const struct reset_control_ops ci_hdrc_msm_reset_ops = {
> + .reset = ci_hdrc_msm_por_reset,
> +};
> +
>  static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
>  {
>   struct device *dev = ci->dev->parent;
> @@ -186,10 +221,18 @@ static int ci_hdrc_msm_probe(struct platform_device 
> *pdev)
>   if (!ci->base)
>   return -ENOMEM;
>  
> - ret = clk_prepare_enable(ci->fs_clk);
> + ci->rcdev.owner = THIS_MODULE;
> + ci->rcdev.ops = _hdrc_msm_reset_ops;
> + ci->rcdev.of_node = pdev->dev.of_node;
> + ci->rcdev.nr_resets = 2;
> + ret = reset_controller_register(>rcdev);
>   if (ret)
>   return ret;
>  
> + ret = clk_prepare_enable(ci->fs_clk);
> + if (ret)
> + goto err_fs;
> +
>   reset_control_assert(reset);
>   usleep_range(1, 12000);
>   reset_control_deassert(reset);
> @@ -198,7 +241,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  
>   ret = clk_prepare_enable(ci->core_clk);
>   if (ret)
> - return ret;
> + goto err_fs;
>  
>   ret = clk_prepare_enable(ci->iface_clk);
>   if (ret)
> @@ -236,6 +279,8 @@ err_mux:
>   clk_disable_unprepare(ci->iface_clk);
>  err_iface:
>   clk_disable_unprepare(ci->core_clk);
> +err_fs:
> + reset_controller_unregister(>rcdev);
>   return ret;
>  }
>  
> @@ -247,6 +292,7 @@ static int ci_hdrc_msm_remove(struct platform_device 
> *pdev)
>   ci_hdrc_remove_device(ci->ci);
>   clk_disable_unprepare(ci->iface_clk);
>   clk_disable_unprepare(ci->core_clk);
> + reset_controller_unregister(>rcdev);
>  
>   return 0;
>  }

Acked-by: Peter Chen 

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 18/22] usb: chipidea: msm: Add reset controller for PHY POR bit

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:32PM -0700, Stephen Boyd wrote:
> The MSM chipidea wrapper has two bits that are used to reset the
> first or second phy. Add support for these bits via the reset
> controller framework, so that phy drivers can reset their
> hardware at the right time during initialization.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/Kconfig   |  1 +
>  drivers/usb/chipidea/ci_hdrc_msm.c | 50 
> --
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index 19c20eaa23f2..fc96f5cdcb5c 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -2,6 +2,7 @@ config USB_CHIPIDEA
>   tristate "ChipIdea Highspeed Dual Role Controller"
>   depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && 
> !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
>   select EXTCON
> + select RESET_CONTROLLER
>   help
> Say Y here if your system has a dual role high speed USB
> controller based on ChipIdea silicon IP. It supports:
> diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c 
> b/drivers/usb/chipidea/ci_hdrc_msm.c
> index 2489a63d3e75..fe96df7b530c 100644
> --- a/drivers/usb/chipidea/ci_hdrc_msm.c
> +++ b/drivers/usb/chipidea/ci_hdrc_msm.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -31,8 +32,10 @@
>  #define HSPHY_SESS_VLD_CTRL  BIT(25)
>  
>  /* Vendor base starts at 0x200 beyond CI base */
> +#define HS_PHY_CTRL  0x0040
>  #define HS_PHY_SEC_CTRL  0x0078
>  #define HS_PHY_DIG_CLAMP_N   BIT(16)
> +#define HS_PHY_POR_ASSERTBIT(0)
>  
>  struct ci_hdrc_msm {
>   struct platform_device *ci;
> @@ -40,11 +43,43 @@ struct ci_hdrc_msm {
>   struct clk *iface_clk;
>   struct clk *fs_clk;
>   struct ci_hdrc_platform_data pdata;
> + struct reset_controller_dev rcdev;
>   bool secondary_phy;
>   bool hsic;
>   void __iomem *base;
>  };
>  
> +static int
> +ci_hdrc_msm_por_reset(struct reset_controller_dev *r, unsigned long id)
> +{
> + struct ci_hdrc_msm *ci_msm = container_of(r, struct ci_hdrc_msm, rcdev);
> + void __iomem *addr = ci_msm->base;
> + u32 val;
> +
> + if (id)
> + addr += HS_PHY_SEC_CTRL;
> + else
> + addr += HS_PHY_CTRL;
> +
> + val = readl_relaxed(addr);
> + val |= HS_PHY_POR_ASSERT;
> + writel(val, addr);
> + /*
> +  * wait for minimum 10 microseconds as suggested by manual.
> +  * Use a slightly larger value since the exact value didn't
> +  * work 100% of the time.
> +  */
> + udelay(12);
> + val &= ~HS_PHY_POR_ASSERT;
> + writel(val, addr);
> +
> + return 0;
> +}
> +
> +static const struct reset_control_ops ci_hdrc_msm_reset_ops = {
> + .reset = ci_hdrc_msm_por_reset,
> +};
> +
>  static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
>  {
>   struct device *dev = ci->dev->parent;
> @@ -186,10 +221,18 @@ static int ci_hdrc_msm_probe(struct platform_device 
> *pdev)
>   if (!ci->base)
>   return -ENOMEM;
>  
> - ret = clk_prepare_enable(ci->fs_clk);
> + ci->rcdev.owner = THIS_MODULE;
> + ci->rcdev.ops = _hdrc_msm_reset_ops;
> + ci->rcdev.of_node = pdev->dev.of_node;
> + ci->rcdev.nr_resets = 2;
> + ret = reset_controller_register(>rcdev);
>   if (ret)
>   return ret;
>  
> + ret = clk_prepare_enable(ci->fs_clk);
> + if (ret)
> + goto err_fs;
> +
>   reset_control_assert(reset);
>   usleep_range(1, 12000);
>   reset_control_deassert(reset);
> @@ -198,7 +241,7 @@ static int ci_hdrc_msm_probe(struct platform_device *pdev)
>  
>   ret = clk_prepare_enable(ci->core_clk);
>   if (ret)
> - return ret;
> + goto err_fs;
>  
>   ret = clk_prepare_enable(ci->iface_clk);
>   if (ret)
> @@ -236,6 +279,8 @@ err_mux:
>   clk_disable_unprepare(ci->iface_clk);
>  err_iface:
>   clk_disable_unprepare(ci->core_clk);
> +err_fs:
> + reset_controller_unregister(>rcdev);
>   return ret;
>  }
>  
> @@ -247,6 +292,7 @@ static int ci_hdrc_msm_remove(struct platform_device 
> *pdev)
>   ci_hdrc_remove_device(ci->ci);
>   clk_disable_unprepare(ci->iface_clk);
>   clk_disable_unprepare(ci->core_clk);
> + reset_controller_unregister(>rcdev);
>  
>   return 0;
>  }

Acked-by: Peter Chen 

-- 

Best Regards,
Peter Chen

[v2 PATCH 2/3] clk: rockchip: add dclk_vop_frac ids for vop

[Chris Zhong]

From: Yakir Yang 

Export the dclk_vop_frac out, so we can set the dclk_vop as the
child of dclk_vop_frac, and then we can start to take use of
the fractional dividers.

Signed-off-by: Xing Zheng 
Signed-off-by: Yakir Yang 
Signed-off-by: Chris Zhong 
---

 include/dt-bindings/clock/rk3399-cru.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/rk3399-cru.h 
b/include/dt-bindings/clock/rk3399-cru.h
index ce5f3e9..220a60f 100644
--- a/include/dt-bindings/clock/rk3399-cru.h
+++ b/include/dt-bindings/clock/rk3399-cru.h
@@ -138,6 +138,8 @@
 #define DCLK_VOP0_DIV  182
 #define DCLK_VOP1_DIV  183
 #define DCLK_M0_PERILP 184
+#define DCLK_VOP0_FRAC 185
+#define DCLK_VOP1_FRAC 186
 
 #define FCLK_CM0S  190
 
-- 
1.9.1

[v2 PATCH 2/3] clk: rockchip: add dclk_vop_frac ids for vop

[Chris Zhong]

From: Yakir Yang 

Export the dclk_vop_frac out, so we can set the dclk_vop as the
child of dclk_vop_frac, and then we can start to take use of
the fractional dividers.

Signed-off-by: Xing Zheng 
Signed-off-by: Yakir Yang 
Signed-off-by: Chris Zhong 
---

 include/dt-bindings/clock/rk3399-cru.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/clock/rk3399-cru.h 
b/include/dt-bindings/clock/rk3399-cru.h
index ce5f3e9..220a60f 100644
--- a/include/dt-bindings/clock/rk3399-cru.h
+++ b/include/dt-bindings/clock/rk3399-cru.h
@@ -138,6 +138,8 @@
 #define DCLK_VOP0_DIV  182
 #define DCLK_VOP1_DIV  183
 #define DCLK_M0_PERILP 184
+#define DCLK_VOP0_FRAC 185
+#define DCLK_VOP1_FRAC 186
 
 #define FCLK_CM0S  190
 
-- 
1.9.1

Re: [PATCH v3 10/22] usb: chipidea: Consolidate extcon notifiers

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
> The two extcon notifiers are almost the same except for the
> variable name for the cable structure and the id notifier inverts
> the cable->state logic. Make it the same and replace two
> functions with one to save some lines. This also makes it so that
> the id cable state is true when the id pin is pulled low.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: "Ivan T. Ivanov" 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/core.c | 41 ++---
>  drivers/usb/chipidea/otg.c  |  4 ++--
>  2 files changed, 12 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index f144e1bbcc82..f3b8d7488648 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -577,35 +577,14 @@ static irqreturn_t ci_irq(int irq, void *data)
>   return ret;
>  }
>  
> -static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
> - void *ptr)
> +static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> +  void *ptr)
>  {
> - struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
> - struct ci_hdrc *ci = vbus->ci;
> + struct ci_hdrc_cable *cbl = container_of(nb, struct ci_hdrc_cable, nb);
> + struct ci_hdrc *ci = cbl->ci;
>  
> - if (event)
> - vbus->state = true;
> - else
> - vbus->state = false;
> -
> - vbus->changed = true;
> -
> - ci_irq(ci->irq, ci);
> - return NOTIFY_DONE;
> -}
> -
> -static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
> -   void *ptr)
> -{
> - struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
> - struct ci_hdrc *ci = id->ci;
> -
> - if (event)
> - id->state = false;
> - else
> - id->state = true;
> -
> - id->changed = true;
> + cbl->state = event;
> + cbl->changed = true;
>  
>   ci_irq(ci->irq, ci);
>   return NOTIFY_DONE;
> @@ -714,7 +693,7 @@ static int ci_get_platdata(struct device *dev,
>   }
>  
>   cable = >vbus_extcon;
> - cable->nb.notifier_call = ci_vbus_notifier;
> + cable->nb.notifier_call = ci_cable_notifier;
>   cable->edev = ext_vbus;
>  
>   if (!IS_ERR(ext_vbus)) {
> @@ -726,15 +705,15 @@ static int ci_get_platdata(struct device *dev,
>   }
>  
>   cable = >id_extcon;
> - cable->nb.notifier_call = ci_id_notifier;
> + cable->nb.notifier_call = ci_cable_notifier;
>   cable->edev = ext_id;
>  
>   if (!IS_ERR(ext_id)) {
>   ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
>   if (ret)
> - cable->state = false;
> - else
>   cable->state = true;
> + else
> + cable->state = false;
>   }
>   return 0;
>  }
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 0cf149e8..fb58d6b312c2 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -63,9 +63,9 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
>   val &= ~OTGSC_IDIS;
>  
>   if (cable->state)
> - val |= OTGSC_ID;
> + val &= ~OTGSC_ID; /* A device */
>   else
> - val &= ~OTGSC_ID;
> + val |= OTGSC_ID; /* B device */
>  
>   if (cable->enabled)
>   val |= OTGSC_IDIE;

/**
 * struct ci_hdrc_cable - structure for external connector cable state tracking
 * @state: current state of the line

You may change the name of variable "state" to "connected", per I
understand, it has changed to the meaning of connected status for your patch.

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 10/22] usb: chipidea: Consolidate extcon notifiers

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:24PM -0700, Stephen Boyd wrote:
> The two extcon notifiers are almost the same except for the
> variable name for the cable structure and the id notifier inverts
> the cable->state logic. Make it the same and replace two
> functions with one to save some lines. This also makes it so that
> the id cable state is true when the id pin is pulled low.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: "Ivan T. Ivanov" 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/core.c | 41 ++---
>  drivers/usb/chipidea/otg.c  |  4 ++--
>  2 files changed, 12 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index f144e1bbcc82..f3b8d7488648 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -577,35 +577,14 @@ static irqreturn_t ci_irq(int irq, void *data)
>   return ret;
>  }
>  
> -static int ci_vbus_notifier(struct notifier_block *nb, unsigned long event,
> - void *ptr)
> +static int ci_cable_notifier(struct notifier_block *nb, unsigned long event,
> +  void *ptr)
>  {
> - struct ci_hdrc_cable *vbus = container_of(nb, struct ci_hdrc_cable, nb);
> - struct ci_hdrc *ci = vbus->ci;
> + struct ci_hdrc_cable *cbl = container_of(nb, struct ci_hdrc_cable, nb);
> + struct ci_hdrc *ci = cbl->ci;
>  
> - if (event)
> - vbus->state = true;
> - else
> - vbus->state = false;
> -
> - vbus->changed = true;
> -
> - ci_irq(ci->irq, ci);
> - return NOTIFY_DONE;
> -}
> -
> -static int ci_id_notifier(struct notifier_block *nb, unsigned long event,
> -   void *ptr)
> -{
> - struct ci_hdrc_cable *id = container_of(nb, struct ci_hdrc_cable, nb);
> - struct ci_hdrc *ci = id->ci;
> -
> - if (event)
> - id->state = false;
> - else
> - id->state = true;
> -
> - id->changed = true;
> + cbl->state = event;
> + cbl->changed = true;
>  
>   ci_irq(ci->irq, ci);
>   return NOTIFY_DONE;
> @@ -714,7 +693,7 @@ static int ci_get_platdata(struct device *dev,
>   }
>  
>   cable = >vbus_extcon;
> - cable->nb.notifier_call = ci_vbus_notifier;
> + cable->nb.notifier_call = ci_cable_notifier;
>   cable->edev = ext_vbus;
>  
>   if (!IS_ERR(ext_vbus)) {
> @@ -726,15 +705,15 @@ static int ci_get_platdata(struct device *dev,
>   }
>  
>   cable = >id_extcon;
> - cable->nb.notifier_call = ci_id_notifier;
> + cable->nb.notifier_call = ci_cable_notifier;
>   cable->edev = ext_id;
>  
>   if (!IS_ERR(ext_id)) {
>   ret = extcon_get_cable_state_(cable->edev, EXTCON_USB_HOST);
>   if (ret)
> - cable->state = false;
> - else
>   cable->state = true;
> + else
> + cable->state = false;
>   }
>   return 0;
>  }
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 0cf149e8..fb58d6b312c2 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -63,9 +63,9 @@ u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask)
>   val &= ~OTGSC_IDIS;
>  
>   if (cable->state)
> - val |= OTGSC_ID;
> + val &= ~OTGSC_ID; /* A device */
>   else
> - val &= ~OTGSC_ID;
> + val |= OTGSC_ID; /* B device */
>  
>   if (cable->enabled)
>   val |= OTGSC_IDIE;

/**
 * struct ci_hdrc_cable - structure for external connector cable state tracking
 * @state: current state of the line

You may change the name of variable "state" to "connected", per I
understand, it has changed to the meaning of connected status for your patch.

-- 

Best Regards,
Peter Chen

[v2 PATCH 3/3] clk: rockchip: use the dclk_vop_frac clock ids

[Chris Zhong]

From: Yakir Yang 

Export the dclk_vop_frac out, so we can set the dclk_vop as the
child of dclk_vop_frac, and then we can start to take use of
the fractional dividers.

Signed-off-by: Xing Zheng 
Signed-off-by: Yakir Yang 
Signed-off-by: Chris Zhong 
---

 drivers/clk/rockchip/clk-rk3399.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index 59417c5..2c7cba7 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1168,7 +1168,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKSEL_CON(49), 8, 2, MFLAGS, 0, 8, DFLAGS,
RK3399_CLKGATE_CON(10), 12, GFLAGS),
 
-   COMPOSITE_FRACMUX_NOGATE(0, "dclk_vop0_frac", "dclk_vop0_div", 0,
+   COMPOSITE_FRACMUX_NOGATE(DCLK_VOP0_FRAC, "dclk_vop0_frac", 
"dclk_vop0_div", 0,
RK3399_CLKSEL_CON(106), 0,
_dclk_vop0_fracmux),
 
@@ -1198,7 +1198,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 8, DFLAGS,
RK3399_CLKGATE_CON(10), 13, GFLAGS),
 
-   COMPOSITE_FRACMUX_NOGATE(0, "dclk_vop1_frac", "dclk_vop1_div", 0,
+   COMPOSITE_FRACMUX_NOGATE(DCLK_VOP1_FRAC, "dclk_vop1_frac", 
"dclk_vop1_div", 0,
RK3399_CLKSEL_CON(107), 0,
_dclk_vop1_fracmux),
 
-- 
1.9.1

[v2 PATCH 1/3] clk: rockchip: Fractional dividers can't set parent rates

[Chris Zhong]

From: Douglas Anderson 

Currently the fractional divider clock time can't handle the
CLK_SET_RATE_PARENT flag. This is because, unlike normal dividers,
there is no clk_divider_bestdiv() function to try speeding up the parent
to see if it helps things.

Eventually someone could try to figure out how to make fractional
dividers able to use CLK_SET_RATE_PARENT, but until they do let's not
confuse the common clock framework (and anyone using it) by setting the
flag.

Signed-off-by: Douglas Anderson 
Signed-off-by: Chris Zhong 
---

 drivers/clk/rockchip/clk-rk3399.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index ea32b7e..59417c5 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -585,7 +585,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_spdif_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(32), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 13, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_spdif_frac", "clk_spdif_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_spdif_frac", "clk_spdif_div", 0,
RK3399_CLKSEL_CON(99), 0,
RK3399_CLKGATE_CON(8), 14, GFLAGS,
_spdif_fracmux),
@@ -599,7 +599,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s0_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(28), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 3, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s0_frac", "clk_i2s0_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s0_frac", "clk_i2s0_div", 0,
RK3399_CLKSEL_CON(96), 0,
RK3399_CLKGATE_CON(8), 4, GFLAGS,
_i2s0_fracmux),
@@ -609,7 +609,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s1_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(29), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 6, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s1_frac", "clk_i2s1_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s1_frac", "clk_i2s1_div", 0,
RK3399_CLKSEL_CON(97), 0,
RK3399_CLKGATE_CON(8), 7, GFLAGS,
_i2s1_fracmux),
@@ -619,7 +619,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s2_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(30), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 9, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s2_frac", "clk_i2s2_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s2_frac", "clk_i2s2_div", 0,
RK3399_CLKSEL_CON(98), 0,
RK3399_CLKGATE_CON(8), 10, GFLAGS,
_i2s2_fracmux),
@@ -638,7 +638,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart0_div", "clk_uart0_src", 0,
RK3399_CLKSEL_CON(33), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 0, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart0_frac", "clk_uart0_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart0_frac", "clk_uart0_div", 0,
RK3399_CLKSEL_CON(100), 0,
RK3399_CLKGATE_CON(9), 1, GFLAGS,
_uart0_fracmux),
@@ -648,7 +648,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart1_div", "clk_uart_src", 0,
RK3399_CLKSEL_CON(34), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 2, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart1_frac", "clk_uart1_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart1_frac", "clk_uart1_div", 0,
RK3399_CLKSEL_CON(101), 0,
RK3399_CLKGATE_CON(9), 3, GFLAGS,
_uart1_fracmux),
@@ -656,7 +656,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart2_div", "clk_uart_src", 0,
RK3399_CLKSEL_CON(35), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 4, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart2_frac", "clk_uart2_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart2_frac", "clk_uart2_div", 0,
RK3399_CLKSEL_CON(102), 0,
RK3399_CLKGATE_CON(9), 5, GFLAGS,

[v2 PATCH 3/3] clk: rockchip: use the dclk_vop_frac clock ids

[Chris Zhong]

From: Yakir Yang 

Export the dclk_vop_frac out, so we can set the dclk_vop as the
child of dclk_vop_frac, and then we can start to take use of
the fractional dividers.

Signed-off-by: Xing Zheng 
Signed-off-by: Yakir Yang 
Signed-off-by: Chris Zhong 
---

 drivers/clk/rockchip/clk-rk3399.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index 59417c5..2c7cba7 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -1168,7 +1168,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKSEL_CON(49), 8, 2, MFLAGS, 0, 8, DFLAGS,
RK3399_CLKGATE_CON(10), 12, GFLAGS),
 
-   COMPOSITE_FRACMUX_NOGATE(0, "dclk_vop0_frac", "dclk_vop0_div", 0,
+   COMPOSITE_FRACMUX_NOGATE(DCLK_VOP0_FRAC, "dclk_vop0_frac", 
"dclk_vop0_div", 0,
RK3399_CLKSEL_CON(106), 0,
_dclk_vop0_fracmux),
 
@@ -1198,7 +1198,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
RK3399_CLKSEL_CON(50), 8, 2, MFLAGS, 0, 8, DFLAGS,
RK3399_CLKGATE_CON(10), 13, GFLAGS),
 
-   COMPOSITE_FRACMUX_NOGATE(0, "dclk_vop1_frac", "dclk_vop1_div", 0,
+   COMPOSITE_FRACMUX_NOGATE(DCLK_VOP1_FRAC, "dclk_vop1_frac", 
"dclk_vop1_div", 0,
RK3399_CLKSEL_CON(107), 0,
_dclk_vop1_fracmux),
 
-- 
1.9.1

[v2 PATCH 1/3] clk: rockchip: Fractional dividers can't set parent rates

[Chris Zhong]

From: Douglas Anderson 

Currently the fractional divider clock time can't handle the
CLK_SET_RATE_PARENT flag. This is because, unlike normal dividers,
there is no clk_divider_bestdiv() function to try speeding up the parent
to see if it helps things.

Eventually someone could try to figure out how to make fractional
dividers able to use CLK_SET_RATE_PARENT, but until they do let's not
confuse the common clock framework (and anyone using it) by setting the
flag.

Signed-off-by: Douglas Anderson 
Signed-off-by: Chris Zhong 
---

 drivers/clk/rockchip/clk-rk3399.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c 
b/drivers/clk/rockchip/clk-rk3399.c
index ea32b7e..59417c5 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -585,7 +585,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_spdif_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(32), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 13, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_spdif_frac", "clk_spdif_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_spdif_frac", "clk_spdif_div", 0,
RK3399_CLKSEL_CON(99), 0,
RK3399_CLKGATE_CON(8), 14, GFLAGS,
_spdif_fracmux),
@@ -599,7 +599,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s0_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(28), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 3, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s0_frac", "clk_i2s0_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s0_frac", "clk_i2s0_div", 0,
RK3399_CLKSEL_CON(96), 0,
RK3399_CLKGATE_CON(8), 4, GFLAGS,
_i2s0_fracmux),
@@ -609,7 +609,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s1_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(29), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 6, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s1_frac", "clk_i2s1_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s1_frac", "clk_i2s1_div", 0,
RK3399_CLKSEL_CON(97), 0,
RK3399_CLKGATE_CON(8), 7, GFLAGS,
_i2s1_fracmux),
@@ -619,7 +619,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE(0, "clk_i2s2_div", mux_pll_src_cpll_gpll_p, 0,
RK3399_CLKSEL_CON(30), 7, 1, MFLAGS, 0, 7, DFLAGS,
RK3399_CLKGATE_CON(8), 9, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_i2s2_frac", "clk_i2s2_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_i2s2_frac", "clk_i2s2_div", 0,
RK3399_CLKSEL_CON(98), 0,
RK3399_CLKGATE_CON(8), 10, GFLAGS,
_i2s2_fracmux),
@@ -638,7 +638,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart0_div", "clk_uart0_src", 0,
RK3399_CLKSEL_CON(33), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 0, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart0_frac", "clk_uart0_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart0_frac", "clk_uart0_div", 0,
RK3399_CLKSEL_CON(100), 0,
RK3399_CLKGATE_CON(9), 1, GFLAGS,
_uart0_fracmux),
@@ -648,7 +648,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart1_div", "clk_uart_src", 0,
RK3399_CLKSEL_CON(34), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 2, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart1_frac", "clk_uart1_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart1_frac", "clk_uart1_div", 0,
RK3399_CLKSEL_CON(101), 0,
RK3399_CLKGATE_CON(9), 3, GFLAGS,
_uart1_fracmux),
@@ -656,7 +656,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] 
__initdata = {
COMPOSITE_NOMUX(0, "clk_uart2_div", "clk_uart_src", 0,
RK3399_CLKSEL_CON(35), 0, 7, DFLAGS,
RK3399_CLKGATE_CON(9), 4, GFLAGS),
-   COMPOSITE_FRACMUX(0, "clk_uart2_frac", "clk_uart2_div", 
CLK_SET_RATE_PARENT,
+   COMPOSITE_FRACMUX(0, "clk_uart2_frac", "clk_uart2_div", 0,
RK3399_CLKSEL_CON(102), 0,
RK3399_CLKGATE_CON(9), 5, GFLAGS,
_uart2_fracmux),
@@ -664,7 +664,7 @@ static struct rockchip_clk_branch 

[PATCH v5 4/4] nvmem: rockchip-efuse: add rk3399-efuse support

[Finley Xiao]

1) the efuse timing of rk3399 is different from earlier SoCs.
2) rk3399-efuse is organized as 32bits by 32 one-time programmable
electrical fuses, the efuse of earlier SoCs is organized as 32bits
by 8 one-time programmable electrical fuses with random access interface.

This patch adds a new read function for rk3399-efuse.

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 drivers/nvmem/rockchip-efuse.c | 133 +++--
 1 file changed, 114 insertions(+), 19 deletions(-)

diff --git a/drivers/nvmem/rockchip-efuse.c b/drivers/nvmem/rockchip-efuse.c
index 4d3f391..423907b 100644
--- a/drivers/nvmem/rockchip-efuse.c
+++ b/drivers/nvmem/rockchip-efuse.c
@@ -22,17 +22,29 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
-#define EFUSE_A_SHIFT  6
-#define EFUSE_A_MASK   0x3ff
-#define EFUSE_PGENBBIT(3)
-#define EFUSE_LOAD BIT(2)
-#define EFUSE_STROBE   BIT(1)
-#define EFUSE_CSB  BIT(0)
-
-#define REG_EFUSE_CTRL 0x
-#define REG_EFUSE_DOUT 0x0004
+#define RK3288_A_SHIFT 6
+#define RK3288_A_MASK  0x3ff
+#define RK3288_PGENB   BIT(3)
+#define RK3288_LOADBIT(2)
+#define RK3288_STROBE  BIT(1)
+#define RK3288_CSB BIT(0)
+
+#define RK3399_A_SHIFT 16
+#define RK3399_A_MASK  0x3ff
+#define RK3399_NBYTES  4
+#define RK3399_STROBSFTSEL BIT(9)
+#define RK3399_RSB BIT(7)
+#define RK3399_PD  BIT(5)
+#define RK3399_PGENB   BIT(3)
+#define RK3399_LOADBIT(2)
+#define RK3399_STROBE  BIT(1)
+#define RK3399_CSB BIT(0)
+
+#define REG_EFUSE_CTRL 0x
+#define REG_EFUSE_DOUT 0x0004
 
 struct rockchip_efuse_chip {
struct device *dev;
@@ -40,8 +52,8 @@ struct rockchip_efuse_chip {
struct clk *clk;
 };
 
-static int rockchip_efuse_read(void *context, unsigned int offset,
-  void *val, size_t bytes)
+static int rockchip_rk3288_efuse_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
 {
struct rockchip_efuse_chip *efuse = context;
u8 *buf = val;
@@ -53,27 +65,82 @@ static int rockchip_efuse_read(void *context, unsigned int 
offset,
return ret;
}
 
-   writel(EFUSE_LOAD | EFUSE_PGENB, efuse->base + REG_EFUSE_CTRL);
+   writel(RK3288_LOAD | RK3288_PGENB, efuse->base + REG_EFUSE_CTRL);
udelay(1);
while (bytes--) {
writel(readl(efuse->base + REG_EFUSE_CTRL) &
-(~(EFUSE_A_MASK << EFUSE_A_SHIFT)),
+(~(RK3288_A_MASK << RK3288_A_SHIFT)),
 efuse->base + REG_EFUSE_CTRL);
writel(readl(efuse->base + REG_EFUSE_CTRL) |
-((offset++ & EFUSE_A_MASK) << EFUSE_A_SHIFT),
+((offset++ & RK3288_A_MASK) << RK3288_A_SHIFT),
 efuse->base + REG_EFUSE_CTRL);
udelay(1);
writel(readl(efuse->base + REG_EFUSE_CTRL) |
-EFUSE_STROBE, efuse->base + REG_EFUSE_CTRL);
+RK3288_STROBE, efuse->base + REG_EFUSE_CTRL);
udelay(1);
*buf++ = readb(efuse->base + REG_EFUSE_DOUT);
writel(readl(efuse->base + REG_EFUSE_CTRL) &
-(~EFUSE_STROBE), efuse->base + REG_EFUSE_CTRL);
+  (~RK3288_STROBE), efuse->base + REG_EFUSE_CTRL);
+   udelay(1);
+   }
+
+   /* Switch to standby mode */
+   writel(RK3288_PGENB | RK3288_CSB, efuse->base + REG_EFUSE_CTRL);
+
+   clk_disable_unprepare(efuse->clk);
+
+   return 0;
+}
+
+static int rockchip_rk3399_efuse_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+   struct rockchip_efuse_chip *efuse = context;
+   unsigned int addr_start, addr_end, addr_offset, addr_len;
+   u32 out_value;
+   u8 *buf;
+   int ret, i = 0;
+
+   ret = clk_prepare_enable(efuse->clk);
+   if (ret < 0) {
+   dev_err(efuse->dev, "failed to prepare/enable efuse clk\n");
+   return ret;
+   }
+
+   addr_start = rounddown(offset, RK3399_NBYTES) / RK3399_NBYTES;
+   addr_end = roundup(offset + bytes, RK3399_NBYTES) / RK3399_NBYTES;
+   addr_offset = offset % RK3399_NBYTES;
+   addr_len = addr_end - addr_start;
+
+   buf = kzalloc(sizeof(*buf) * addr_len * RK3399_NBYTES, GFP_KERNEL);
+   if (!buf) {
+   clk_disable_unprepare(efuse->clk);
+   return -ENOMEM;
+   }
+
+   writel(RK3399_LOAD | RK3399_PGENB | RK3399_STROBSFTSEL | RK3399_RSB,
+  efuse->base 

[PATCH v5 1/4] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Finley Xiao]

Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
programmable electrical fuses with random access interface.

Add different device tree compatible string for different SoCs to be able
to differentiate between the two. The old binding is of course preserved,
though deprecated.

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt 
b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
index 8f86ab3..94aeeea 100644
--- a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
@@ -1,11 +1,20 @@
 = Rockchip eFuse device tree bindings =
 
 Required properties:
-- compatible: Should be "rockchip,rockchip-efuse"
+- compatible: Should be one of the following.
+  - "rockchip,rk3066a-efuse" - for RK3066a SoCs.
+  - "rockchip,rk3188-efuse" - for RK3188 SoCs.
+  - "rockchip,rk3288-efuse" - for RK3288 SoCs.
+  - "rockchip,rk3399-efuse" - for RK3399 SoCs.
 - reg: Should contain the registers location and exact eFuse size
 - clocks: Should be the clock id of eFuse
 - clock-names: Should be "pclk_efuse"
 
+Deprecated properties:
+- compatible: "rockchip,rockchip-efuse"
+  Old efuse compatible value compatible to rk3066a, rk3188 and rk3288
+  efuses
+
 = Data cells =
 Are child nodes of eFuse, bindings of which as described in
 bindings/nvmem/nvmem.txt
@@ -13,7 +22,7 @@ bindings/nvmem/nvmem.txt
 Example:
 
efuse: efuse@ffb4 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3288-efuse";
reg = <0xffb4 0x20>;
#address-cells = <1>;
#size-cells = <1>;
-- 
2.7.4

[PATCH v5 4/4] nvmem: rockchip-efuse: add rk3399-efuse support

[Finley Xiao]

1) the efuse timing of rk3399 is different from earlier SoCs.
2) rk3399-efuse is organized as 32bits by 32 one-time programmable
electrical fuses, the efuse of earlier SoCs is organized as 32bits
by 8 one-time programmable electrical fuses with random access interface.

This patch adds a new read function for rk3399-efuse.

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 drivers/nvmem/rockchip-efuse.c | 133 +++--
 1 file changed, 114 insertions(+), 19 deletions(-)

diff --git a/drivers/nvmem/rockchip-efuse.c b/drivers/nvmem/rockchip-efuse.c
index 4d3f391..423907b 100644
--- a/drivers/nvmem/rockchip-efuse.c
+++ b/drivers/nvmem/rockchip-efuse.c
@@ -22,17 +22,29 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
-#define EFUSE_A_SHIFT  6
-#define EFUSE_A_MASK   0x3ff
-#define EFUSE_PGENBBIT(3)
-#define EFUSE_LOAD BIT(2)
-#define EFUSE_STROBE   BIT(1)
-#define EFUSE_CSB  BIT(0)
-
-#define REG_EFUSE_CTRL 0x
-#define REG_EFUSE_DOUT 0x0004
+#define RK3288_A_SHIFT 6
+#define RK3288_A_MASK  0x3ff
+#define RK3288_PGENB   BIT(3)
+#define RK3288_LOADBIT(2)
+#define RK3288_STROBE  BIT(1)
+#define RK3288_CSB BIT(0)
+
+#define RK3399_A_SHIFT 16
+#define RK3399_A_MASK  0x3ff
+#define RK3399_NBYTES  4
+#define RK3399_STROBSFTSEL BIT(9)
+#define RK3399_RSB BIT(7)
+#define RK3399_PD  BIT(5)
+#define RK3399_PGENB   BIT(3)
+#define RK3399_LOADBIT(2)
+#define RK3399_STROBE  BIT(1)
+#define RK3399_CSB BIT(0)
+
+#define REG_EFUSE_CTRL 0x
+#define REG_EFUSE_DOUT 0x0004
 
 struct rockchip_efuse_chip {
struct device *dev;
@@ -40,8 +52,8 @@ struct rockchip_efuse_chip {
struct clk *clk;
 };
 
-static int rockchip_efuse_read(void *context, unsigned int offset,
-  void *val, size_t bytes)
+static int rockchip_rk3288_efuse_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
 {
struct rockchip_efuse_chip *efuse = context;
u8 *buf = val;
@@ -53,27 +65,82 @@ static int rockchip_efuse_read(void *context, unsigned int 
offset,
return ret;
}
 
-   writel(EFUSE_LOAD | EFUSE_PGENB, efuse->base + REG_EFUSE_CTRL);
+   writel(RK3288_LOAD | RK3288_PGENB, efuse->base + REG_EFUSE_CTRL);
udelay(1);
while (bytes--) {
writel(readl(efuse->base + REG_EFUSE_CTRL) &
-(~(EFUSE_A_MASK << EFUSE_A_SHIFT)),
+(~(RK3288_A_MASK << RK3288_A_SHIFT)),
 efuse->base + REG_EFUSE_CTRL);
writel(readl(efuse->base + REG_EFUSE_CTRL) |
-((offset++ & EFUSE_A_MASK) << EFUSE_A_SHIFT),
+((offset++ & RK3288_A_MASK) << RK3288_A_SHIFT),
 efuse->base + REG_EFUSE_CTRL);
udelay(1);
writel(readl(efuse->base + REG_EFUSE_CTRL) |
-EFUSE_STROBE, efuse->base + REG_EFUSE_CTRL);
+RK3288_STROBE, efuse->base + REG_EFUSE_CTRL);
udelay(1);
*buf++ = readb(efuse->base + REG_EFUSE_DOUT);
writel(readl(efuse->base + REG_EFUSE_CTRL) &
-(~EFUSE_STROBE), efuse->base + REG_EFUSE_CTRL);
+  (~RK3288_STROBE), efuse->base + REG_EFUSE_CTRL);
+   udelay(1);
+   }
+
+   /* Switch to standby mode */
+   writel(RK3288_PGENB | RK3288_CSB, efuse->base + REG_EFUSE_CTRL);
+
+   clk_disable_unprepare(efuse->clk);
+
+   return 0;
+}
+
+static int rockchip_rk3399_efuse_read(void *context, unsigned int offset,
+ void *val, size_t bytes)
+{
+   struct rockchip_efuse_chip *efuse = context;
+   unsigned int addr_start, addr_end, addr_offset, addr_len;
+   u32 out_value;
+   u8 *buf;
+   int ret, i = 0;
+
+   ret = clk_prepare_enable(efuse->clk);
+   if (ret < 0) {
+   dev_err(efuse->dev, "failed to prepare/enable efuse clk\n");
+   return ret;
+   }
+
+   addr_start = rounddown(offset, RK3399_NBYTES) / RK3399_NBYTES;
+   addr_end = roundup(offset + bytes, RK3399_NBYTES) / RK3399_NBYTES;
+   addr_offset = offset % RK3399_NBYTES;
+   addr_len = addr_end - addr_start;
+
+   buf = kzalloc(sizeof(*buf) * addr_len * RK3399_NBYTES, GFP_KERNEL);
+   if (!buf) {
+   clk_disable_unprepare(efuse->clk);
+   return -ENOMEM;
+   }
+
+   writel(RK3399_LOAD | RK3399_PGENB | RK3399_STROBSFTSEL | RK3399_RSB,
+  efuse->base + REG_EFUSE_CTRL);
+   udelay(1);
+  

[PATCH v5 1/4] nvmem: rockchip-efuse: update compatible strings for Rockchip efuse

[Finley Xiao]

Rk3399-efuse is organized as 32bits by 32 one-time programmable electrical
fuses. The efuse of earlier SoCs are organized as 32bits by 8 one-time
programmable electrical fuses with random access interface.

Add different device tree compatible string for different SoCs to be able
to differentiate between the two. The old binding is of course preserved,
though deprecated.

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt 
b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
index 8f86ab3..94aeeea 100644
--- a/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
+++ b/Documentation/devicetree/bindings/nvmem/rockchip-efuse.txt
@@ -1,11 +1,20 @@
 = Rockchip eFuse device tree bindings =
 
 Required properties:
-- compatible: Should be "rockchip,rockchip-efuse"
+- compatible: Should be one of the following.
+  - "rockchip,rk3066a-efuse" - for RK3066a SoCs.
+  - "rockchip,rk3188-efuse" - for RK3188 SoCs.
+  - "rockchip,rk3288-efuse" - for RK3288 SoCs.
+  - "rockchip,rk3399-efuse" - for RK3399 SoCs.
 - reg: Should contain the registers location and exact eFuse size
 - clocks: Should be the clock id of eFuse
 - clock-names: Should be "pclk_efuse"
 
+Deprecated properties:
+- compatible: "rockchip,rockchip-efuse"
+  Old efuse compatible value compatible to rk3066a, rk3188 and rk3288
+  efuses
+
 = Data cells =
 Are child nodes of eFuse, bindings of which as described in
 bindings/nvmem/nvmem.txt
@@ -13,7 +22,7 @@ bindings/nvmem/nvmem.txt
 Example:
 
efuse: efuse@ffb4 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3288-efuse";
reg = <0xffb4 0x20>;
#address-cells = <1>;
#size-cells = <1>;
-- 
2.7.4

[PATCH v5 2/4] ARM: dts: rockchip: update compatible strings for Rockchip efuse

[Finley Xiao]

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 arch/arm/boot/dts/rk3066a.dtsi | 2 +-
 arch/arm/boot/dts/rk3188.dtsi  | 2 +-
 arch/arm/boot/dts/rk3288.dtsi  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index c0ba86c..5387cc8 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -162,7 +162,7 @@
};
 
efuse: efuse@2001 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3066a-efuse";
reg = <0x2001 0x4000>;
#address-cells = <1>;
#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..869e189 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -147,7 +147,7 @@
};
 
efuse: efuse@2001 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3188-efuse";
reg = <0x2001 0x4000>;
#address-cells = <1>;
#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index cd33f01..0eadb96 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1073,7 +1073,7 @@
};
 
efuse: efuse@ffb4 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3288-efuse";
reg = <0xffb4 0x20>;
#address-cells = <1>;
#size-cells = <1>;
-- 
2.7.4

[PATCH v5 3/4] arm64: dts: rockchip: add efuse0 device node for rk3399

[Finley Xiao]

Add a efuse0 node in the device tree for the ARM64 rk3399 SoC.

Signed-off-by: Finley Xiao 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 29 +
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index a6dd623..05b48e2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -721,6 +721,35 @@
status = "disabled";
};
 
+   efuse0: efuse@ff69 {
+   compatible = "rockchip,rk3399-efuse";
+   reg = <0x0 0xff69 0x0 0x80>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   clocks = < PCLK_EFUSE1024NS>;
+   clock-names = "pclk_efuse";
+
+   /* Data cells */
+   cpul_leakage: cpul-leakage {
+   reg = <0x1a 0x1>;
+   };
+   cpub_leakage: cpub-leakage {
+   reg = <0x17 0x1>;
+   };
+   gpu_leakage: gpu-leakage {
+   reg = <0x18 0x1>;
+   };
+   center_leakage: center-leakage {
+   reg = <0x19 0x1>;
+   };
+   logic_leakage: logic-leakage {
+   reg = <0x1b 0x1>;
+   };
+   wafer_info: wafer-info {
+   reg = <0x1c 0x1>;
+   };
+   };
+
pmucru: pmu-clock-controller@ff75 {
compatible = "rockchip,rk3399-pmucru";
reg = <0x0 0xff75 0x0 0x1000>;
-- 
2.7.4

[PATCH v5 0/4] nvmem: rockchip-efuse: support more rockchip SoCs

[Finley Xiao]

As the timing and organization of efuse may be different  
between rockchip SoCs, so their read function may be different.   
We add different device tree compatible string for rockchip SoCs  
to match their own read function. 
  
V4->V5:   
- 3/4 alter the efuse node name
- 4/4 align the macro definition
  disable clk if fail to allocate memory

Finley Xiao (4):
  nvmem: rockchip-efuse: update compatible strings for Rockchip efuse
  ARM: dts: rockchip: update compatible strings for Rockchip efuse
  arm64: dts: rockchip: add efuse0 device node for rk3399
  nvmem: rockchip-efuse: add rk3399-efuse support

 .../devicetree/bindings/nvmem/rockchip-efuse.txt   |  13 +-
 arch/arm/boot/dts/rk3066a.dtsi |   2 +-
 arch/arm/boot/dts/rk3188.dtsi  |   2 +-
 arch/arm/boot/dts/rk3288.dtsi  |   2 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |  29 +
 drivers/nvmem/rockchip-efuse.c | 133 ++---
 6 files changed, 157 insertions(+), 24 deletions(-)

-- 
2.7.4

[PATCH v5 2/4] ARM: dts: rockchip: update compatible strings for Rockchip efuse

[Finley Xiao]

Signed-off-by: Finley Xiao 
Reviewed-by: Heiko Stuebner 
---
 arch/arm/boot/dts/rk3066a.dtsi | 2 +-
 arch/arm/boot/dts/rk3188.dtsi  | 2 +-
 arch/arm/boot/dts/rk3288.dtsi  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/rk3066a.dtsi b/arch/arm/boot/dts/rk3066a.dtsi
index c0ba86c..5387cc8 100644
--- a/arch/arm/boot/dts/rk3066a.dtsi
+++ b/arch/arm/boot/dts/rk3066a.dtsi
@@ -162,7 +162,7 @@
};
 
efuse: efuse@2001 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3066a-efuse";
reg = <0x2001 0x4000>;
#address-cells = <1>;
#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3188.dtsi b/arch/arm/boot/dts/rk3188.dtsi
index 31f81b2..869e189 100644
--- a/arch/arm/boot/dts/rk3188.dtsi
+++ b/arch/arm/boot/dts/rk3188.dtsi
@@ -147,7 +147,7 @@
};
 
efuse: efuse@2001 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3188-efuse";
reg = <0x2001 0x4000>;
#address-cells = <1>;
#size-cells = <1>;
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index cd33f01..0eadb96 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1073,7 +1073,7 @@
};
 
efuse: efuse@ffb4 {
-   compatible = "rockchip,rockchip-efuse";
+   compatible = "rockchip,rk3288-efuse";
reg = <0xffb4 0x20>;
#address-cells = <1>;
#size-cells = <1>;
-- 
2.7.4

[PATCH v5 3/4] arm64: dts: rockchip: add efuse0 device node for rk3399

[Finley Xiao]

Add a efuse0 node in the device tree for the ARM64 rk3399 SoC.

Signed-off-by: Finley Xiao 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 29 +
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index a6dd623..05b48e2 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -721,6 +721,35 @@
status = "disabled";
};
 
+   efuse0: efuse@ff69 {
+   compatible = "rockchip,rk3399-efuse";
+   reg = <0x0 0xff69 0x0 0x80>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   clocks = < PCLK_EFUSE1024NS>;
+   clock-names = "pclk_efuse";
+
+   /* Data cells */
+   cpul_leakage: cpul-leakage {
+   reg = <0x1a 0x1>;
+   };
+   cpub_leakage: cpub-leakage {
+   reg = <0x17 0x1>;
+   };
+   gpu_leakage: gpu-leakage {
+   reg = <0x18 0x1>;
+   };
+   center_leakage: center-leakage {
+   reg = <0x19 0x1>;
+   };
+   logic_leakage: logic-leakage {
+   reg = <0x1b 0x1>;
+   };
+   wafer_info: wafer-info {
+   reg = <0x1c 0x1>;
+   };
+   };
+
pmucru: pmu-clock-controller@ff75 {
compatible = "rockchip,rk3399-pmucru";
reg = <0x0 0xff75 0x0 0x1000>;
-- 
2.7.4

[PATCH v5 0/4] nvmem: rockchip-efuse: support more rockchip SoCs

[Finley Xiao]

As the timing and organization of efuse may be different  
between rockchip SoCs, so their read function may be different.   
We add different device tree compatible string for rockchip SoCs  
to match their own read function. 
  
V4->V5:   
- 3/4 alter the efuse node name
- 4/4 align the macro definition
  disable clk if fail to allocate memory

Finley Xiao (4):
  nvmem: rockchip-efuse: update compatible strings for Rockchip efuse
  ARM: dts: rockchip: update compatible strings for Rockchip efuse
  arm64: dts: rockchip: add efuse0 device node for rk3399
  nvmem: rockchip-efuse: add rk3399-efuse support

 .../devicetree/bindings/nvmem/rockchip-efuse.txt   |  13 +-
 arch/arm/boot/dts/rk3066a.dtsi |   2 +-
 arch/arm/boot/dts/rk3188.dtsi  |   2 +-
 arch/arm/boot/dts/rk3288.dtsi  |   2 +-
 arch/arm64/boot/dts/rockchip/rk3399.dtsi   |  29 +
 drivers/nvmem/rockchip-efuse.c | 133 ++---
 6 files changed, 157 insertions(+), 24 deletions(-)

-- 
2.7.4

Re: [PATCH v3 09/22] usb: chipidea: Add support for ULPI PHY bus

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:23PM -0700, Stephen Boyd wrote:
> Some phys for the chipidea controller are controlled via the ULPI
> viewport. Add support for the ULPI bus so that these sorts of
> phys can be probed and read/written automatically without having
> to duplicate the viewport logic in each phy driver.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: Heikki Krogerus 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/Kconfig  |   7 +++
>  drivers/usb/chipidea/Makefile |   1 +
>  drivers/usb/chipidea/ci.h |  21 
>  drivers/usb/chipidea/core.c   |  31 +---
>  drivers/usb/chipidea/ulpi.c   | 113 
> ++
>  5 files changed, 167 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/usb/chipidea/ulpi.c
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index 5e5b9eb7ebf6..19c20eaa23f2 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -38,4 +38,11 @@ config USB_CHIPIDEA_HOST
> Say Y here to enable host controller functionality of the
> ChipIdea driver.
>  
> +config USB_CHIPIDEA_ULPI
> + bool "ChipIdea ULPI PHY support"
> + depends on USB_ULPI_BUS=y || USB_ULPI_BUS=USB_CHIPIDEA
> + help
> +   Say Y here if you have a ULPI PHY attached to your ChipIdea
> +   controller.
> +
>  endif
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 518e445476c3..39fca5715ed3 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -4,6 +4,7 @@ ci_hdrc-y := core.o otg.o debug.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC)   += udc.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)  += host.o
>  ci_hdrc-$(CONFIG_USB_OTG_FSM)+= otg_fsm.o
> +ci_hdrc-$(CONFIG_USB_CHIPIDEA_ULPI)  += ulpi.o
>  
>  # Glue/Bridge layers go here
>  
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 05bc4d631cb9..59e22389c10b 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -18,6 +18,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  
> /**
>   * DEFINE
> @@ -52,6 +54,7 @@ enum ci_hw_regs {
>   OP_ENDPTLISTADDR,
>   OP_TTCTRL,
>   OP_BURSTSIZE,
> + OP_ULPI_VIEWPORT,
>   OP_PORTSC,
>   OP_DEVLC,
>   OP_OTGSC,
> @@ -187,6 +190,8 @@ struct hw_bank {
>   * @test_mode: the selected test mode
>   * @platdata: platform specific information supplied by parent device
>   * @vbus_active: is VBUS active
> + * @ulpi: pointer to ULPI device, if any
> + * @ulpi_ops: ULPI read/write ops for this device
>   * @phy: pointer to PHY, if any
>   * @usb_phy: pointer to USB PHY, if any and if using the USB PHY framework
>   * @hcd: pointer to usb_hcd for ehci host driver
> @@ -236,6 +241,10 @@ struct ci_hdrc {
>  
>   struct ci_hdrc_platform_data*platdata;
>   int vbus_active;
> +#ifdef CONFIG_USB_CHIPIDEA_ULPI
> + struct ulpi *ulpi;
> + struct ulpi_ops ulpi_ops;
> +#endif
>   struct phy  *phy;
>   /* old usb_phy interface */
>   struct usb_phy  *usb_phy;
> @@ -418,6 +427,16 @@ static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
>  #endif
>  }
>  
> +#if IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI)
> +int ci_ulpi_init(struct ci_hdrc *ci);
> +void ci_ulpi_exit(struct ci_hdrc *ci);
> +int ci_ulpi_resume(struct ci_hdrc *ci);
> +#else
> +static inline int ci_ulpi_init(struct ci_hdrc *ci) { return 0; }
> +static inline void ci_ulpi_exit(struct ci_hdrc *ci) { }
> +static inline int ci_ulpi_resume(struct ci_hdrc *ci) { return 0; }
> +#endif
> +
>  u32 hw_read_intr_enable(struct ci_hdrc *ci);
>  
>  u32 hw_read_intr_status(struct ci_hdrc *ci);
> @@ -428,6 +447,8 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> +void hw_phymode_configure(struct ci_hdrc *ci);
> +
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 532085a096d9..f144e1bbcc82 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -86,6 +86,7 @@ static const u8 ci_regs_nolpm[] = {
>   [OP_ENDPTLISTADDR]  = 0x18U,
>   [OP_TTCTRL] = 0x1CU,
>   [OP_BURSTSIZE]  = 0x20U,
> + [OP_ULPI_VIEWPORT]  = 0x30U,
>   [OP_PORTSC] = 0x44U,
>   [OP_DEVLC]  = 0x84U,
>   [OP_OTGSC]  = 0x64U,
> @@ -110,6 +111,7 @@ static const u8 ci_regs_lpm[] = {
>   [OP_ENDPTLISTADDR]  = 0x18U,
>   [OP_TTCTRL] = 0x1CU,
>   

Re: [PATCH v3 09/22] usb: chipidea: Add support for ULPI PHY bus

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:23PM -0700, Stephen Boyd wrote:
> Some phys for the chipidea controller are controlled via the ULPI
> viewport. Add support for the ULPI bus so that these sorts of
> phys can be probed and read/written automatically without having
> to duplicate the viewport logic in each phy driver.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: Heikki Krogerus 
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/Kconfig  |   7 +++
>  drivers/usb/chipidea/Makefile |   1 +
>  drivers/usb/chipidea/ci.h |  21 
>  drivers/usb/chipidea/core.c   |  31 +---
>  drivers/usb/chipidea/ulpi.c   | 113 
> ++
>  5 files changed, 167 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/usb/chipidea/ulpi.c
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
> index 5e5b9eb7ebf6..19c20eaa23f2 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -38,4 +38,11 @@ config USB_CHIPIDEA_HOST
> Say Y here to enable host controller functionality of the
> ChipIdea driver.
>  
> +config USB_CHIPIDEA_ULPI
> + bool "ChipIdea ULPI PHY support"
> + depends on USB_ULPI_BUS=y || USB_ULPI_BUS=USB_CHIPIDEA
> + help
> +   Say Y here if you have a ULPI PHY attached to your ChipIdea
> +   controller.
> +
>  endif
> diff --git a/drivers/usb/chipidea/Makefile b/drivers/usb/chipidea/Makefile
> index 518e445476c3..39fca5715ed3 100644
> --- a/drivers/usb/chipidea/Makefile
> +++ b/drivers/usb/chipidea/Makefile
> @@ -4,6 +4,7 @@ ci_hdrc-y := core.o otg.o debug.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_UDC)   += udc.o
>  ci_hdrc-$(CONFIG_USB_CHIPIDEA_HOST)  += host.o
>  ci_hdrc-$(CONFIG_USB_OTG_FSM)+= otg_fsm.o
> +ci_hdrc-$(CONFIG_USB_CHIPIDEA_ULPI)  += ulpi.o
>  
>  # Glue/Bridge layers go here
>  
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 05bc4d631cb9..59e22389c10b 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -18,6 +18,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  
> /**
>   * DEFINE
> @@ -52,6 +54,7 @@ enum ci_hw_regs {
>   OP_ENDPTLISTADDR,
>   OP_TTCTRL,
>   OP_BURSTSIZE,
> + OP_ULPI_VIEWPORT,
>   OP_PORTSC,
>   OP_DEVLC,
>   OP_OTGSC,
> @@ -187,6 +190,8 @@ struct hw_bank {
>   * @test_mode: the selected test mode
>   * @platdata: platform specific information supplied by parent device
>   * @vbus_active: is VBUS active
> + * @ulpi: pointer to ULPI device, if any
> + * @ulpi_ops: ULPI read/write ops for this device
>   * @phy: pointer to PHY, if any
>   * @usb_phy: pointer to USB PHY, if any and if using the USB PHY framework
>   * @hcd: pointer to usb_hcd for ehci host driver
> @@ -236,6 +241,10 @@ struct ci_hdrc {
>  
>   struct ci_hdrc_platform_data*platdata;
>   int vbus_active;
> +#ifdef CONFIG_USB_CHIPIDEA_ULPI
> + struct ulpi *ulpi;
> + struct ulpi_ops ulpi_ops;
> +#endif
>   struct phy  *phy;
>   /* old usb_phy interface */
>   struct usb_phy  *usb_phy;
> @@ -418,6 +427,16 @@ static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
>  #endif
>  }
>  
> +#if IS_ENABLED(CONFIG_USB_CHIPIDEA_ULPI)
> +int ci_ulpi_init(struct ci_hdrc *ci);
> +void ci_ulpi_exit(struct ci_hdrc *ci);
> +int ci_ulpi_resume(struct ci_hdrc *ci);
> +#else
> +static inline int ci_ulpi_init(struct ci_hdrc *ci) { return 0; }
> +static inline void ci_ulpi_exit(struct ci_hdrc *ci) { }
> +static inline int ci_ulpi_resume(struct ci_hdrc *ci) { return 0; }
> +#endif
> +
>  u32 hw_read_intr_enable(struct ci_hdrc *ci);
>  
>  u32 hw_read_intr_status(struct ci_hdrc *ci);
> @@ -428,6 +447,8 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> +void hw_phymode_configure(struct ci_hdrc *ci);
> +
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 532085a096d9..f144e1bbcc82 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -86,6 +86,7 @@ static const u8 ci_regs_nolpm[] = {
>   [OP_ENDPTLISTADDR]  = 0x18U,
>   [OP_TTCTRL] = 0x1CU,
>   [OP_BURSTSIZE]  = 0x20U,
> + [OP_ULPI_VIEWPORT]  = 0x30U,
>   [OP_PORTSC] = 0x44U,
>   [OP_DEVLC]  = 0x84U,
>   [OP_OTGSC]  = 0x64U,
> @@ -110,6 +111,7 @@ static const u8 ci_regs_lpm[] = {
>   [OP_ENDPTLISTADDR]  = 0x18U,
>   [OP_TTCTRL] = 0x1CU,
>   [OP_BURSTSIZE]  = 0x20U,
> + [OP_ULPI_VIEWPORT]  = 0x30U,
>   [OP_PORTSC] = 0x44U,
>

Re: DAX can not work on virtual nvdimm device

[Ross Zwisler]

On Wed, Aug 31, 2016 at 04:44:47PM +0800, Xiao Guangrong wrote:
> On 08/31/2016 01:09 AM, Dan Williams wrote:
> > 
> > Can you post your exact reproduction steps?  This test is not failing for 
> > me.
> > 
> 
> Sure.
> 
> 1. make the guest kernel based on your tree, the top commit is
>10d7902fa0e82b (dax: unmap/truncate on device shutdown) and
>the config file can be found in this thread.
> 
> 2. add guest kernel command line: memmap=6G!10G
> 
> 3: start the guest:
>x86_64-softmmu/qemu-system-x86_64 -machine pc,nvdimm --enable-kvm \
>-smp 16 -m 32G,maxmem=100G,slots=100 /other/VMs/centos6.img -monitor stdio
> 
> 4: in guest:
>mkfs.ext4 /dev/pmem0
>mount -o dax /dev/pmem0  /mnt/pmem/
>echo > /mnt/pmem/xxx
>./mmap /mnt/pmem/xxx
>./read /mnt/pmem/xxx
> 
>   The source code of mmap and read has been attached in this mail.
> 
>   Hopefully, you can detect the error triggered by read test.
> 
> Thanks!

Okay, I think I've isolated this issue.  Xiao's VM was an old CentOS 6 system,
and for some reason ext4+DAX with the old tools found in that VM fails.  I was
able to reproduce this failure with a freshly installed CentOS 6.8 VM.

You can see the failure with his tests, or perhaps more easily with this
series of commands:

  # mkfs.ext4 /dev/pmem0
  # mount -o dax /dev/pmem0  /mnt/pmem/
  # touch /mnt/pmem/x
  # md5sum /mnt/pmem/x
  md5sum: /mnt/pmem/x: Bad address

This sequence of commands works fine in the old CentOS 6 system if you use XFS
instead of ext4, and it works fine with both ext4 and XFS in CentOS 7 and
with recent versions of Fedora.

I've added the ext4 folks to this mail in case they care, but my guess is that
the tools in CentOS 6 are so old that it's not worth worrying about.  For
reference, the kernel in CentOS 6 is based on 2.6.32.  :)  DAX was introduced
in v4.0.

Re: DAX can not work on virtual nvdimm device

[Ross Zwisler]

On Wed, Aug 31, 2016 at 04:44:47PM +0800, Xiao Guangrong wrote:
> On 08/31/2016 01:09 AM, Dan Williams wrote:
> > 
> > Can you post your exact reproduction steps?  This test is not failing for 
> > me.
> > 
> 
> Sure.
> 
> 1. make the guest kernel based on your tree, the top commit is
>10d7902fa0e82b (dax: unmap/truncate on device shutdown) and
>the config file can be found in this thread.
> 
> 2. add guest kernel command line: memmap=6G!10G
> 
> 3: start the guest:
>x86_64-softmmu/qemu-system-x86_64 -machine pc,nvdimm --enable-kvm \
>-smp 16 -m 32G,maxmem=100G,slots=100 /other/VMs/centos6.img -monitor stdio
> 
> 4: in guest:
>mkfs.ext4 /dev/pmem0
>mount -o dax /dev/pmem0  /mnt/pmem/
>echo > /mnt/pmem/xxx
>./mmap /mnt/pmem/xxx
>./read /mnt/pmem/xxx
> 
>   The source code of mmap and read has been attached in this mail.
> 
>   Hopefully, you can detect the error triggered by read test.
> 
> Thanks!

Okay, I think I've isolated this issue.  Xiao's VM was an old CentOS 6 system,
and for some reason ext4+DAX with the old tools found in that VM fails.  I was
able to reproduce this failure with a freshly installed CentOS 6.8 VM.

You can see the failure with his tests, or perhaps more easily with this
series of commands:

  # mkfs.ext4 /dev/pmem0
  # mount -o dax /dev/pmem0  /mnt/pmem/
  # touch /mnt/pmem/x
  # md5sum /mnt/pmem/x
  md5sum: /mnt/pmem/x: Bad address

This sequence of commands works fine in the old CentOS 6 system if you use XFS
instead of ext4, and it works fine with both ext4 and XFS in CentOS 7 and
with recent versions of Fedora.

I've added the ext4 folks to this mail in case they care, but my guess is that
the tools in CentOS 6 are so old that it's not worth worrying about.  For
reference, the kernel in CentOS 6 is based on 2.6.32.  :)  DAX was introduced
in v4.0.

Re: [PATCH v3 04/22] usb: chipidea: Only read/write OTGSC from one place

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:18PM -0700, Stephen Boyd wrote:
> With the id and vbus detection done via extcon we need to make
> sure we poll the status of OTGSC properly by considering what the
> extcon is saying, and not just what the register is saying. Let's
> move this hw_wait_reg() function to the only place it's used and
> simplify it for polling the OTGSC register. Then we can make
> certain we only use the hw_read_otgsc() API to read OTGSC, which
> will make sure we properly handle extcon events.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: "Ivan T. Ivanov" 
> Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID 
> detect")
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/ci.h   |  3 ---
>  drivers/usb/chipidea/core.c | 32 
>  drivers/usb/chipidea/otg.c  | 34 ++
>  3 files changed, 30 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index cd414559040f..05bc4d631cb9 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms);
> -
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e644d17..01390e02ee53 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
>   return 0;
>  }
>  
> -/**
> - * hw_wait_reg: wait the register value
> - *
> - * Sometimes, it needs to wait register value before going on.
> - * Eg, when switch to device mode, the vbus value should be lower
> - * than OTGSC_BSV before connects to host.
> - *
> - * @ci: the controller
> - * @reg: register index
> - * @mask: mast bit
> - * @value: the bit value to wait
> - * @timeout_ms: timeout in millisecond
> - *
> - * This function returns an error code if timeout
> - */
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms)
> -{
> - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
> -
> - while (hw_read(ci, reg, mask) != value) {
> - if (time_after(jiffies, elapse)) {
> - dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> - mask, reg);
> - return -ETIMEDOUT;
> - }
> - msleep(20);
> - }
> -
> - return 0;
> -}
> -
>  static irqreturn_t ci_irq(int irq, void *data)
>  {
>   struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 03b6743461d1..a829607c3e4d 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -104,7 +104,31 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
>   usb_gadget_vbus_disconnect(>gadget);
>  }
>  
> -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
> +/**
> + * When we switch to device mode, the vbus value should be lower
> + * than OTGSC_BSV before connecting to host.
> + *
> + * @ci: the controller
> + *
> + * This function returns an error code if timeout
> + */
> +static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
> +{
> + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> + u32 mask = OTGSC_BSV;
> +
> + while (hw_read_otgsc(ci, mask)) {
> + if (time_after(jiffies, elapse)) {
> + dev_err(ci->dev, "timeout waiting for %08x in OTGSC\n",
> + mask);
> + return -ETIMEDOUT;
> + }
> + msleep(20);
> + }
> +
> + return 0;
> +}
> +
>  static void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>   enum ci_role role = ci_otg_role(ci);
> @@ -116,9 +140,11 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
>   ci_role_stop(ci);
>  
>   if (role == CI_ROLE_GADGET)
> - /* wait vbus lower than OTGSC_BSV */
> - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> - CI_VBUS_STABLE_TIMEOUT_MS);
> + /*
> +  * wait vbus lower than OTGSC_BSV before connecting
> +  * to host
> +  */
> + hw_wait_vbus_lower_bsv(ci);
>  
>   ci_role_start(ci, role);
>   }

Acked-by: Peter Chen 

-- 

Best Regards,
Peter Chen

Re: [PATCH v3 04/22] usb: chipidea: Only read/write OTGSC from one place

[Peter Chen]

On Wed, Aug 31, 2016 at 05:40:18PM -0700, Stephen Boyd wrote:
> With the id and vbus detection done via extcon we need to make
> sure we poll the status of OTGSC properly by considering what the
> extcon is saying, and not just what the register is saying. Let's
> move this hw_wait_reg() function to the only place it's used and
> simplify it for polling the OTGSC register. Then we can make
> certain we only use the hw_read_otgsc() API to read OTGSC, which
> will make sure we properly handle extcon events.
> 
> Cc: Peter Chen 
> Cc: Greg Kroah-Hartman 
> Cc: "Ivan T. Ivanov" 
> Fixes: 3ecb3e09b042 ("usb: chipidea: Use extcon framework for VBUS and ID 
> detect")
> Signed-off-by: Stephen Boyd 
> ---
>  drivers/usb/chipidea/ci.h   |  3 ---
>  drivers/usb/chipidea/core.c | 32 
>  drivers/usb/chipidea/otg.c  | 34 ++
>  3 files changed, 30 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index cd414559040f..05bc4d631cb9 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms);
> -
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e644d17..01390e02ee53 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
>   return 0;
>  }
>  
> -/**
> - * hw_wait_reg: wait the register value
> - *
> - * Sometimes, it needs to wait register value before going on.
> - * Eg, when switch to device mode, the vbus value should be lower
> - * than OTGSC_BSV before connects to host.
> - *
> - * @ci: the controller
> - * @reg: register index
> - * @mask: mast bit
> - * @value: the bit value to wait
> - * @timeout_ms: timeout in millisecond
> - *
> - * This function returns an error code if timeout
> - */
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> - u32 value, unsigned int timeout_ms)
> -{
> - unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
> -
> - while (hw_read(ci, reg, mask) != value) {
> - if (time_after(jiffies, elapse)) {
> - dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> - mask, reg);
> - return -ETIMEDOUT;
> - }
> - msleep(20);
> - }
> -
> - return 0;
> -}
> -
>  static irqreturn_t ci_irq(int irq, void *data)
>  {
>   struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 03b6743461d1..a829607c3e4d 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -104,7 +104,31 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
>   usb_gadget_vbus_disconnect(>gadget);
>  }
>  
> -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
> +/**
> + * When we switch to device mode, the vbus value should be lower
> + * than OTGSC_BSV before connecting to host.
> + *
> + * @ci: the controller
> + *
> + * This function returns an error code if timeout
> + */
> +static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
> +{
> + unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> + u32 mask = OTGSC_BSV;
> +
> + while (hw_read_otgsc(ci, mask)) {
> + if (time_after(jiffies, elapse)) {
> + dev_err(ci->dev, "timeout waiting for %08x in OTGSC\n",
> + mask);
> + return -ETIMEDOUT;
> + }
> + msleep(20);
> + }
> +
> + return 0;
> +}
> +
>  static void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>   enum ci_role role = ci_otg_role(ci);
> @@ -116,9 +140,11 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
>   ci_role_stop(ci);
>  
>   if (role == CI_ROLE_GADGET)
> - /* wait vbus lower than OTGSC_BSV */
> - hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> - CI_VBUS_STABLE_TIMEOUT_MS);
> + /*
> +  * wait vbus lower than OTGSC_BSV before connecting
> +  * to host
> +  */
> + hw_wait_vbus_lower_bsv(ci);
>  
>   ci_role_start(ci, role);
>   }

Acked-by: Peter Chen 

-- 

Best Regards,
Peter Chen

Re: [PATCH] powerpc: Clean up tm_abort duplication in hash_utils_64.c

[Rui Teng]

On 9/1/16 11:46 PM, Thiago Jung Bauermann wrote:

Am Freitag, 26 August 2016, 11:50:10 schrieb Rui Teng:

The same logic appears twice and should probably be pulled out into a
function.

Suggested-by: Michael Ellerman 
Signed-off-by: Rui Teng 
---
 arch/powerpc/mm/hash_utils_64.c | 45
+ 1 file changed, 19
insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/mm/hash_utils_64.c
b/arch/powerpc/mm/hash_utils_64.c index 0821556..69ef702 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1460,6 +1460,23 @@ out_exit:
local_irq_restore(flags);
 }

+/*
+ * Transactions are not aborted by tlbiel, only tlbie.
+ * Without, syncing a page back to a block device w/ PIO could pick up
+ * transactional data (bad!) so we force an abort here.  Before the
+ * sync the page will be made read-only, which will flush_hash_page.
+ * BIG ISSUE here: if the kernel uses a page from userspace without
+ * unmapping it first, it may see the speculated version.
+ */
+void local_tm_abort(int local)
+{
+   if (local && cpu_has_feature(CPU_FTR_TM) && current->thread.regs &&
+   MSR_TM_ACTIVE(current->thread.regs->msr)) {
+   tm_enable();
+   tm_abort(TM_CAUSE_TLBI);
+   }
+}
+


Since local_tm_abort is only used in this file, it should be static.

OK


Also, since both places calling it are guarded by
CONFIG_PPC_TRANSACTIONAL_MEM, wouldn't it be cleaner if the #ifdef was here
instead and the #else block defined an empty static inline function? Then
the call sites wouldn't need to be guarded.

I have considered this style before, but I am worried about the call
stacks increased by empty function and forgot the inline function.
Will send v2 with your comments.

Thanks!

Re: [PATCH] powerpc: Clean up tm_abort duplication in hash_utils_64.c

[Rui Teng]

On 9/1/16 11:46 PM, Thiago Jung Bauermann wrote:

Am Freitag, 26 August 2016, 11:50:10 schrieb Rui Teng:

The same logic appears twice and should probably be pulled out into a
function.

Suggested-by: Michael Ellerman 
Signed-off-by: Rui Teng 
---
 arch/powerpc/mm/hash_utils_64.c | 45
+ 1 file changed, 19
insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/mm/hash_utils_64.c
b/arch/powerpc/mm/hash_utils_64.c index 0821556..69ef702 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1460,6 +1460,23 @@ out_exit:
local_irq_restore(flags);
 }

+/*
+ * Transactions are not aborted by tlbiel, only tlbie.
+ * Without, syncing a page back to a block device w/ PIO could pick up
+ * transactional data (bad!) so we force an abort here.  Before the
+ * sync the page will be made read-only, which will flush_hash_page.
+ * BIG ISSUE here: if the kernel uses a page from userspace without
+ * unmapping it first, it may see the speculated version.
+ */
+void local_tm_abort(int local)
+{
+   if (local && cpu_has_feature(CPU_FTR_TM) && current->thread.regs &&
+   MSR_TM_ACTIVE(current->thread.regs->msr)) {
+   tm_enable();
+   tm_abort(TM_CAUSE_TLBI);
+   }
+}
+


Since local_tm_abort is only used in this file, it should be static.

OK


Also, since both places calling it are guarded by
CONFIG_PPC_TRANSACTIONAL_MEM, wouldn't it be cleaner if the #ifdef was here
instead and the #else block defined an empty static inline function? Then
the call sites wouldn't need to be guarded.

I have considered this style before, but I am worried about the call
stacks increased by empty function and forgot the inline function.
Will send v2 with your comments.

Thanks!

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

On 2016年09月02日 05:29, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu  wrote:

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're used
for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
frequency to play.

I think we don't need to store it due to it's a fixed value at run-time,
even if it is reset after a power cycle, the above will not be changed via
software, except for dirver unbind .


I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.

So are you saying that the entire purpose of "corecfg_clockmultiplier"
is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
register to get a certain value?
...and that the entire purpose of "corecfg_baseclkfreq" is that it
causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
to get a certain value?

Yes, on rk3399:
corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock

If you re-write to either corecfg_* stuff,  the corresponding CAP 
register field will be changed too.
sdhci driver will fetch CAP register for initialization, we only need to 
guarantee they're correct at probe time.


Did that all make sense?


That would have been nice to know before.  I had assumed that those
"corecfg" settings did something else more useful.

If it is indeed true that these corecfg values are as silly as it
seems, then I guess it's not terribly important to re-set them after
suspend/resume.  Eventually it would be nice/clean to actually do so
(in case the SDHCI driver eventually changes), but I guess we wouldn't
need to block. this patch from landing.

Can you please confirm my understanding above?


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

On 2016年09月02日 05:29, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xu  wrote:

Hi


On 2016年09月01日 12:20, Doug Anderson wrote:

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works
fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?


Take corecfg_clockmultiplier as example.
1. sdhci driver fetch host->clk_mul from corecfg_clockmultiplier
2. mmc->f_min and mmc->f_max are calculated via host->clk_mul, they're used
for further initialization.
3. if the corecfg_clockmultiplier is incorrect, sdhci will use improper
frequency to play.

I think we don't need to store it due to it's a fixed value at run-time,
even if it is reset after a power cycle, the above will not be changed via
software, except for dirver unbind .


I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?


Yup, corecfg_* stuff will be reset after a power cycle.
I mean that we need only to guarantee they're correct at probe time.

So are you saying that the entire purpose of "corecfg_clockmultiplier"
is that causes the "ClockMultiplier" field of the "EMMCCORE_CAP"
register to get a certain value?
...and that the entire purpose of "corecfg_baseclkfreq" is that it
causes the "BaseClockFreqSDClock" field of the "EMMCCORE_CAP" register
to get a certain value?

Yes, on rk3399:
corecfg_clockmultiplier <===> EMMCCORE_CAP1[23:16] ClockMultiplier
corecfg_baseclkfreq <===> EMMCCORE_CAP[15:8] BaseClockFreqSDClock

If you re-write to either corecfg_* stuff,  the corresponding CAP 
register field will be changed too.
sdhci driver will fetch CAP register for initialization, we only need to 
guarantee they're correct at probe time.


Did that all make sense?


That would have been nice to know before.  I had assumed that those
"corecfg" settings did something else more useful.

If it is indeed true that these corecfg values are as silly as it
seems, then I guess it's not terribly important to re-set them after
suspend/resume.  Eventually it would be nice/clean to actually do so
(in case the SDHCI driver eventually changes), but I guess we wouldn't
need to block. this patch from landing.

Can you please confirm my understanding above?


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

Hi Arnd

在 2016/9/1 22:02, Arnd Bergmann 写道:



2. We need to backward compatible with the old dt way config access as below 
code,
so we have to call hisi_pcie_common_cfg_read() when accessing the RC config 
space.
For this, we have to call hisi_pcie_common_cfg_read().

drivers/pci/host/pcie-hisi.c
static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
int size, u32 *val)
{
struct hisi_pcie *pcie = to_hisi_pcie(pp);

return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}

static struct pcie_host_ops hisi_pcie_host_ops = {
.rd_own_conf = hisi_pcie_cfg_read,
.wr_own_conf = hisi_pcie_cfg_write,
.link_up = hisi_pcie_link_up,
};


I think this would be easier if you separate the ACPI code from the
DT code and not try to have a common file used for both.

Sharing the config space accessors really isn't worth it when both
variants are fairly simple to do, but they don't fit in a common
model because one is called from the ACPI quirks and the other
is called from the dw-pcie driver with completely different calling
conventions.


I agree, many thanks.

Thanks
Dongdong


ARnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

.

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

Hi Arnd

在 2016/9/1 22:02, Arnd Bergmann 写道:



2. We need to backward compatible with the old dt way config access as below 
code,
so we have to call hisi_pcie_common_cfg_read() when accessing the RC config 
space.
For this, we have to call hisi_pcie_common_cfg_read().

drivers/pci/host/pcie-hisi.c
static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
int size, u32 *val)
{
struct hisi_pcie *pcie = to_hisi_pcie(pp);

return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}

static struct pcie_host_ops hisi_pcie_host_ops = {
.rd_own_conf = hisi_pcie_cfg_read,
.wr_own_conf = hisi_pcie_cfg_write,
.link_up = hisi_pcie_link_up,
};


I think this would be easier if you separate the ACPI code from the
DT code and not try to have a common file used for both.

Sharing the config space accessors really isn't worth it when both
variants are fairly simple to do, but they don't fit in a common
model because one is called from the ACPI quirks and the other
is called from the dw-pcie driver with completely different calling
conventions.


I agree, many thanks.

Thanks
Dongdong


ARnd
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

.

Re: chipidea: udc: kernel panic in isr_setup_status_phase

[Peter Chen]

On Tue, Aug 30, 2016 at 07:20:45PM +0200, Clemens Gruber wrote:
> On Mon, Aug 29, 2016 at 06:24:03PM +0800, Peter Chen wrote:
> > Would you please measure the voltage of vbus within 1s at below two
> > conditions:
> > 
> > - Just connect cable
> > - Just disconnect cable
> 
> We found out that there was a problem with our hardware design!
> 
> But first, here is the VBUS measurement during cable plug-in.
> https://s17.postimg.org/8ba3rgl6n/linux_kernel_panic_usb_otg_vbus_is_yellow.jpg
> (The yellow signal is USB_OTG_VBUS, please ignore the red one)
> The kernel panic occurs where the slip of paper with the arrow is.
> The signal looks normal to me, I don't think VBUS was the problem.
> 

Do you have other 5V to USB_H1_VBUS? USB PHY needs 5V input voltage
as the source for USB LDO (3.0v), either from OTG or Host 1. I suspect
the lower vbus voltage causes the USB LDO voltage less than 3.0v, then
cause the unstable for USB PHY. If possible, you can connect MAIN 5V
(if it exists) directly to USB_H1_VBUS to see if this problem is fixed.

> The other theory was a GND problem..
> After thorough investigation, we discovered that we connected all
> SHIELD pins of the USB micro OTG connector to the protective-earth
> ground (GND_PE) with a 1 MOhm resistor and a 1 nF cap in parallel.
> We changed that to connect the SHIELD pins to the same internal GND
> as the USB signals have and also replaced the 1 MOhm resistor with a
> 100 Ohm  resistor as seen in the MCIMX6Q-SDB schematics.
> Since that change, the error did not appear again! :)

Great. Can you see the sudden lower for vbus again? If it still exists, it may
be GND issue.

> 
> The only signals I never checked are the differential high speed signals
> (USB_OTG_DN and USB_OTG_DP) because I don't have the necessary equipment
> like active differential probes for very high frequency measurements, ..
> Maybe with the connection of the shield to PE, the cable acted as an
> antenna and interfered with the high speed signals?


> (Although they are differential, so the same noise to both should not
> be a problem? It is still a mystery to me, what was really causing it)
> 

The poor signal may cause the controller has unexpected behavior.

> 
> What do you think about still fixing that kernel panic in case something
> similar happens again for other users?
> If such a situation occurs, should we only avoid the NULL pointer
> dereference or print out an error message and stop the gadget driver?
> 

Avoid NULL pointer deference is necessary. Patch is welcome :)

-- 

Best Regards,
Peter Chen

Re: chipidea: udc: kernel panic in isr_setup_status_phase

[Peter Chen]

On Tue, Aug 30, 2016 at 07:20:45PM +0200, Clemens Gruber wrote:
> On Mon, Aug 29, 2016 at 06:24:03PM +0800, Peter Chen wrote:
> > Would you please measure the voltage of vbus within 1s at below two
> > conditions:
> > 
> > - Just connect cable
> > - Just disconnect cable
> 
> We found out that there was a problem with our hardware design!
> 
> But first, here is the VBUS measurement during cable plug-in.
> https://s17.postimg.org/8ba3rgl6n/linux_kernel_panic_usb_otg_vbus_is_yellow.jpg
> (The yellow signal is USB_OTG_VBUS, please ignore the red one)
> The kernel panic occurs where the slip of paper with the arrow is.
> The signal looks normal to me, I don't think VBUS was the problem.
> 

Do you have other 5V to USB_H1_VBUS? USB PHY needs 5V input voltage
as the source for USB LDO (3.0v), either from OTG or Host 1. I suspect
the lower vbus voltage causes the USB LDO voltage less than 3.0v, then
cause the unstable for USB PHY. If possible, you can connect MAIN 5V
(if it exists) directly to USB_H1_VBUS to see if this problem is fixed.

> The other theory was a GND problem..
> After thorough investigation, we discovered that we connected all
> SHIELD pins of the USB micro OTG connector to the protective-earth
> ground (GND_PE) with a 1 MOhm resistor and a 1 nF cap in parallel.
> We changed that to connect the SHIELD pins to the same internal GND
> as the USB signals have and also replaced the 1 MOhm resistor with a
> 100 Ohm  resistor as seen in the MCIMX6Q-SDB schematics.
> Since that change, the error did not appear again! :)

Great. Can you see the sudden lower for vbus again? If it still exists, it may
be GND issue.

> 
> The only signals I never checked are the differential high speed signals
> (USB_OTG_DN and USB_OTG_DP) because I don't have the necessary equipment
> like active differential probes for very high frequency measurements, ..
> Maybe with the connection of the shield to PE, the cable acted as an
> antenna and interfered with the high speed signals?


> (Although they are differential, so the same noise to both should not
> be a problem? It is still a mystery to me, what was really causing it)
> 

The poor signal may cause the controller has unexpected behavior.

> 
> What do you think about still fixing that kernel panic in case something
> similar happens again for other users?
> If such a situation occurs, should we only avoid the NULL pointer
> dereference or print out an error message and stop the gadget driver?
> 

Avoid NULL pointer deference is necessary. Patch is welcome :)

-- 

Best Regards,
Peter Chen

Re: [PATCH 1/2] xen/x86: Convert to hotplug state machine

[Boris Ostrovsky]

On 08/31/2016 12:15 PM, Sebastian Andrzej Siewior wrote:
> On 2016-08-26 15:37:38 [-0400], Boris Ostrovsky wrote:
>>> If you do find the time, you might manage to rework the code to avoid
>>> using the _nocalls() function. If see this right, you use
>>> xen_setup_vcpu_info_placement() for the init in the first place. This
>>> uses for_each_possible_cpu macro. The cpuhp_setup_state() function would
>>> perform the init for all CPUs before they come up.
>> I am not sure I see what this would buy us.
>>
>> Besides, cpuhp_setup_state() uses for_each_present_cpu().
> Correct. So you would avoid running the init code on CPUs which are
> within the for_each_possible_cpu() set but not in for_each_present_cpu().
>
> Assuming a NUMA box with two CPUs, 8 cores each gives you 32 CPUs in
> Linux with hyper threading. BIOS may report 240 CPUs as the upper limit
> (possible CPUs) but if you never deploy them you don't need to
> initialize them… Should they be plugged physically then the
> for_each_present_cpu() loop will cover them once they come up.
>

That's not going to help Xen guests: all possible CPUs are brought up
right away and then those that are not in use are unplugged.


-boris

Re: [PATCH 1/2] xen/x86: Convert to hotplug state machine

[Boris Ostrovsky]

On 08/31/2016 12:15 PM, Sebastian Andrzej Siewior wrote:
> On 2016-08-26 15:37:38 [-0400], Boris Ostrovsky wrote:
>>> If you do find the time, you might manage to rework the code to avoid
>>> using the _nocalls() function. If see this right, you use
>>> xen_setup_vcpu_info_placement() for the init in the first place. This
>>> uses for_each_possible_cpu macro. The cpuhp_setup_state() function would
>>> perform the init for all CPUs before they come up.
>> I am not sure I see what this would buy us.
>>
>> Besides, cpuhp_setup_state() uses for_each_present_cpu().
> Correct. So you would avoid running the init code on CPUs which are
> within the for_each_possible_cpu() set but not in for_each_present_cpu().
>
> Assuming a NUMA box with two CPUs, 8 cores each gives you 32 CPUs in
> Linux with hyper threading. BIOS may report 240 CPUs as the upper limit
> (possible CPUs) but if you never deploy them you don't need to
> initialize them… Should they be plugged physically then the
> for_each_present_cpu() loop will cover them once they come up.
>

That's not going to help Xen guests: all possible CPUs are brought up
right away and then those that are not in use are unplugged.


-boris

[GIT PULL] clk fixes for v4.8-rc4

[Stephen Boyd]

The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:

  Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
tags/clk-fixes-for-linus

for you to fetch changes up to dc7066c54107255f5f9a11bf3f82417c9b1aef51:

  Merge tag 'v4.8-rockchip-clk-fixes1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip into 
clk-fixes (2016-08-29 17:08:35 -0700)


A collection of small fixes for various SoC vendor clk drivers.


Chen-Yu Tsai (1):
  clk: sunxi-ng: Fix inverted test condition in ccu_helper_wait_for_lock

Chris Zhong (1):
  clk: rockchip: fix rk3399 aclk_vio gate bit

Stephen Boyd (1):
  Merge tag 'v4.8-rockchip-clk-fixes1' of 
git://git.kernel.org/.../mmind/linux-rockchip into clk-fixes

Vince Hsu (1):
  clk: tegra: remove TEGRA_PLL_USE_LOCK for PLLD/PLLD2

Xing Zheng (3):
  clk: rockchip: fix incorrect aclk_emmc source gate bits on rk3399
  clk: rockchip: fix incorrect GATE bits for {c, g}pll_aclk_perihp_src on 
rk3399
  clk: rockchip: mark aclk_emmc_noc as a critical clock on rk3399

Yoshihiro Shimoda (1):
  clk: renesas: r8a7795: Fix SD clocks

 drivers/clk/renesas/r8a7795-cpg-mssr.c |  9 +
 drivers/clk/rockchip/clk-rk3399.c  | 11 ++-
 drivers/clk/sunxi-ng/ccu_common.c  |  2 +-
 drivers/clk/tegra/clk-tegra114.c   |  4 ++--
 4 files changed, 14 insertions(+), 12 deletions(-)

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

[GIT PULL] clk fixes for v4.8-rc4

[Stephen Boyd]

The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:

  Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
tags/clk-fixes-for-linus

for you to fetch changes up to dc7066c54107255f5f9a11bf3f82417c9b1aef51:

  Merge tag 'v4.8-rockchip-clk-fixes1' of 
git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip into 
clk-fixes (2016-08-29 17:08:35 -0700)


A collection of small fixes for various SoC vendor clk drivers.


Chen-Yu Tsai (1):
  clk: sunxi-ng: Fix inverted test condition in ccu_helper_wait_for_lock

Chris Zhong (1):
  clk: rockchip: fix rk3399 aclk_vio gate bit

Stephen Boyd (1):
  Merge tag 'v4.8-rockchip-clk-fixes1' of 
git://git.kernel.org/.../mmind/linux-rockchip into clk-fixes

Vince Hsu (1):
  clk: tegra: remove TEGRA_PLL_USE_LOCK for PLLD/PLLD2

Xing Zheng (3):
  clk: rockchip: fix incorrect aclk_emmc source gate bits on rk3399
  clk: rockchip: fix incorrect GATE bits for {c, g}pll_aclk_perihp_src on 
rk3399
  clk: rockchip: mark aclk_emmc_noc as a critical clock on rk3399

Yoshihiro Shimoda (1):
  clk: renesas: r8a7795: Fix SD clocks

 drivers/clk/renesas/r8a7795-cpg-mssr.c |  9 +
 drivers/clk/rockchip/clk-rk3399.c  | 11 ++-
 drivers/clk/sunxi-ng/ccu_common.c  |  2 +-
 drivers/clk/tegra/clk-tegra114.c   |  4 ++--
 4 files changed, 14 insertions(+), 12 deletions(-)

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

Re: [PATCH 1/7] drm/sun4i: support TCONs without channel 1

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 11:31 PM, Maxime Ripard
 wrote:
> Some Allwinner SoCs, such as the A33, have a variation of the TCON that
> doesn't have a second channel (or it is not wired to anything).
>
> Make sure we can handle that case.
>
> Signed-off-by: Maxime Ripard 

Acked-by: Chen-Yu Tsai 

Re: [PATCH 1/7] drm/sun4i: support TCONs without channel 1

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 11:31 PM, Maxime Ripard
 wrote:
> Some Allwinner SoCs, such as the A33, have a variation of the TCON that
> doesn't have a second channel (or it is not wired to anything).
>
> Make sure we can handle that case.
>
> Signed-off-by: Maxime Ripard 

Acked-by: Chen-Yu Tsai 

Re: [PATCH 4/5] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc

[Chanwoo Choi]

Hi Heiko,

On 2016년 09월 02일 08:17, Heiko Stübner wrote:
> Am Freitag, 2. September 2016, 06:31:24 schrieb Lin Huang:
>> base on dfi result, we do ddr frequency scaling, register
>> dmc driver to devfreq framework, and use simple-ondemand
>> policy.
>>
>> Signed-off-by: Lin Huang 
>> Signed-off-by: MyngJoo Ham 
>> Reviewed-by: Chanwoo Choi 
>> ---
> 
> [...]
> 
>> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
>> new file mode 100644
>> index 000..54d65f2
>> --- /dev/null
>> +++ b/drivers/devfreq/rk3399_dmc.c
>> @@ -0,0 +1,480 @@
>> +/*
>> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd.
>> + * Author: Lin Huang 
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>> for + * more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
> 
> (Repeated from previous version)
> For the shared sip-interface header I've prepared a signed, stable tag
> which can be pulled from
> 
> The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
> 
>   Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git 
> tags/rockchip-ddr-sip
> 
> for you to fetch changes up to 97dd82682f1a6174698fbea149a04b4cabc58c4f:
> 
>   soc: rockchip: add header for ddr rate SIP interface (2016-08-31 18:53:24 
> +0200)
> 
> 
> Header file defining the SIP-interface to the ATF for DDR frequency changes
> 
> 
> Lin Huang (1):
>   soc: rockchip: add header for ddr rate SIP interface
> 
>  include/soc/rockchip/rockchip_sip.h | 27 +++
>  1 file changed, 27 insertions(+)
>  create mode 100644 include/soc/rockchip/rockchip_sip.h
> 
> 
> 

Thanks for creating the immutable branch.

-- 
Best Regards,
Chanwoo Choi

Re: [PATCH 4/5] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc

[Chanwoo Choi]

Hi Heiko,

On 2016년 09월 02일 08:17, Heiko Stübner wrote:
> Am Freitag, 2. September 2016, 06:31:24 schrieb Lin Huang:
>> base on dfi result, we do ddr frequency scaling, register
>> dmc driver to devfreq framework, and use simple-ondemand
>> policy.
>>
>> Signed-off-by: Lin Huang 
>> Signed-off-by: MyngJoo Ham 
>> Reviewed-by: Chanwoo Choi 
>> ---
> 
> [...]
> 
>> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
>> new file mode 100644
>> index 000..54d65f2
>> --- /dev/null
>> +++ b/drivers/devfreq/rk3399_dmc.c
>> @@ -0,0 +1,480 @@
>> +/*
>> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd.
>> + * Author: Lin Huang 
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>> for + * more details.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
> 
> (Repeated from previous version)
> For the shared sip-interface header I've prepared a signed, stable tag
> which can be pulled from
> 
> The following changes since commit 29b4817d4018df78086157ea3a55c1d9424a7cfc:
> 
>   Linux 4.8-rc1 (2016-08-07 18:18:00 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git 
> tags/rockchip-ddr-sip
> 
> for you to fetch changes up to 97dd82682f1a6174698fbea149a04b4cabc58c4f:
> 
>   soc: rockchip: add header for ddr rate SIP interface (2016-08-31 18:53:24 
> +0200)
> 
> 
> Header file defining the SIP-interface to the ATF for DDR frequency changes
> 
> 
> Lin Huang (1):
>   soc: rockchip: add header for ddr rate SIP interface
> 
>  include/soc/rockchip/rockchip_sip.h | 27 +++
>  1 file changed, 27 insertions(+)
>  create mode 100644 include/soc/rockchip/rockchip_sip.h
> 
> 
> 

Thanks for creating the immutable branch.

-- 
Best Regards,
Chanwoo Choi

Re: [linux-sunxi] [PATCH 6/7] ARM: sun8i: a33: Add RGB666 pins

[Chen-Yu Tsai]

Hi,

On Thu, Sep 1, 2016 at 11:32 PM, Maxime Ripard
 wrote:
> The LCD output needs to be muxed. Add the proper pinctrl node.
>
> Signed-off-by: Maxime Ripard 
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi 
> b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 5f9dbd17eb50..d5f93c05846f 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -300,6 +300,16 @@
> interrupts = ,
>  ;
>
> +   tcon0_rgb666_pins_a: tcon0_rgb666@0 {

This is the only possible combination. You can drop the _a and @0.

Also this can be shared with A23, as they are pin compatible, and I
also matched the datasheets.

Otherwise

Acked-by: Chen-Yu Tsai 

> +   allwinner,pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> +"PD10", "PD11", "PD12", "PD13", "PD14", 
> "PD15",
> +"PD18", "PD19", "PD20", "PD21", "PD22", 
> "PD23",
> +"PD24", "PD25", "PD26", "PD27";
> +   allwinner,function = "lcd0";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> uart0_pins_b: uart0@1 {
> allwinner,pins = "PB0", "PB1";
> allwinner,function = "uart0";
> --
> 2.9.2
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [linux-sunxi] [PATCH 6/7] ARM: sun8i: a33: Add RGB666 pins

[Chen-Yu Tsai]

Hi,

On Thu, Sep 1, 2016 at 11:32 PM, Maxime Ripard
 wrote:
> The LCD output needs to be muxed. Add the proper pinctrl node.
>
> Signed-off-by: Maxime Ripard 
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi 
> b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 5f9dbd17eb50..d5f93c05846f 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -300,6 +300,16 @@
> interrupts = ,
>  ;
>
> +   tcon0_rgb666_pins_a: tcon0_rgb666@0 {

This is the only possible combination. You can drop the _a and @0.

Also this can be shared with A23, as they are pin compatible, and I
also matched the datasheets.

Otherwise

Acked-by: Chen-Yu Tsai 

> +   allwinner,pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> +"PD10", "PD11", "PD12", "PD13", "PD14", 
> "PD15",
> +"PD18", "PD19", "PD20", "PD21", "PD22", 
> "PD23",
> +"PD24", "PD25", "PD26", "PD27";
> +   allwinner,function = "lcd0";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> uart0_pins_b: uart0@1 {
> allwinner,pins = "PB0", "PB1";
> allwinner,function = "uart0";
> --
> 2.9.2
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:49PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> >Some hard-wired USB devices need to do power sequence to let the
> >device work normally, the typical power sequence like: enable USB
> >PHY clock, toggle reset pin, etc. But current Linux USB driver
> >lacks of such code to do it, it may cause some hard-wired USB devices
> >works abnormal or can't be recognized by controller at all.
> >
> >In this patch, it calls power sequence library APIs to finish
> >the power sequence events. At first, it calls pwrseq_alloc_generic
> >to create a generic power sequence instance, then it will do power
> >on sequence at hub's probe for all devices under this hub
> >(includes root hub).
> >
> >At hub_disconnect, it will do power off sequence which is at powered
> >on list.
> >
> >Signed-off-by: Peter Chen 
> >Tested-by Joshua Clayton 
> >---
> >  drivers/usb/core/Makefile |   1 +
> >  drivers/usb/core/hub.c|  12 --
> >  drivers/usb/core/hub.h|  12 ++
> >  drivers/usb/core/pwrseq.c | 100 
> > ++
> >  4 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >
> >diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> >index 9780877..39f2149 100644
> >--- a/drivers/usb/core/Makefile
> >+++ b/drivers/usb/core/Makefile
> >@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> >+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
> >  obj-$(CONFIG_USB)  += usbcore.o
> >diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >index bee1351..a346a8b 100644
> >--- a/drivers/usb/core/hub.c
> >+++ b/drivers/usb/core/hub.c
> >@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >+hub_pwrseq_off(hub);
> > mutex_lock(_port_peer_mutex);
> > /* Avoid races with recursively_mark_NOTATTACHED() */
> >@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> >struct usb_device_id *id)
> > struct usb_endpoint_descriptor *endpoint;
> > struct usb_device *hdev;
> > struct usb_hub *hub;
> >+int ret = -ENODEV;
> > desc = intf->cur_altsetting;
> > hdev = interface_to_usbdev(intf);
> >@@ -1839,6 +1841,7 @@ descriptor_error:
> > INIT_DELAYED_WORK(>leds, led_work);
> > INIT_DELAYED_WORK(>init_work, NULL);
> > INIT_WORK(>events, hub_event);
> >+INIT_LIST_HEAD(>pwrseq_on_list);
> > usb_get_intf(intf);
> > usb_get_dev(hdev);
> >@@ -1852,11 +1855,14 @@ descriptor_error:
> > if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
> > hub->quirk_check_port_auto_suspend = 1;
> >-if (hub_configure(hub, endpoint) >= 0)
> >-return 0;
> >+if (hub_configure(hub, endpoint) >= 0) {
> >+ret = hub_pwrseq_on(hub);
> >+if (!ret)
> >+return 0;
> >+}
> > hub_disconnect(intf);
> >-return -ENODEV;
> >+return ret;
> >  }
> >  static int
> >diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> >index 34c1a7e..9473f6f 100644
> >--- a/drivers/usb/core/hub.h
> >+++ b/drivers/usb/core/hub.h
> >@@ -78,6 +78,7 @@ struct usb_hub {
> > struct delayed_work init_work;
> > struct work_struct  events;
> > struct usb_port **ports;
> >+struct list_headpwrseq_on_list; /* powered pwrseq node list */
> >  };
> >  /**
> >@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> >usb_hub *hub,
> >  {
> > return hub_port_debounce(hub, port1, false);
> >  }
> >+
> >+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> >+int hub_pwrseq_on(struct usb_hub *hub);
> >+void hub_pwrseq_off(struct usb_hub *hub);
> >+#else
> >+static inline int hub_pwrseq_on(struct usb_hub *hub)
> >+{
> >+return 0;
> >+}
> >+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> >+#endif /* CONFIG_PWRSEQ_GENERIC */
> >diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> >new file mode 100644
> >index 000..837fe66
> >--- /dev/null
> >+++ b/drivers/usb/core/pwrseq.c
> >@@ -0,0 +1,100 @@
> >+/*
> >+ * pwrseq.c USB device power sequence management
> >+ *
> >+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
> >+ * Author: Peter Chen 
> >+ *
> >+ * This program is free software: you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2  of
> >+ * the License as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more 

Re: [PATCH v6 4/8] usb: core: add power sequence handling for USB devices

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:49PM +0530, Vaibhav Hiremath wrote:
> 
> 
> On Monday 15 August 2016 02:43 PM, Peter Chen wrote:
> >Some hard-wired USB devices need to do power sequence to let the
> >device work normally, the typical power sequence like: enable USB
> >PHY clock, toggle reset pin, etc. But current Linux USB driver
> >lacks of such code to do it, it may cause some hard-wired USB devices
> >works abnormal or can't be recognized by controller at all.
> >
> >In this patch, it calls power sequence library APIs to finish
> >the power sequence events. At first, it calls pwrseq_alloc_generic
> >to create a generic power sequence instance, then it will do power
> >on sequence at hub's probe for all devices under this hub
> >(includes root hub).
> >
> >At hub_disconnect, it will do power off sequence which is at powered
> >on list.
> >
> >Signed-off-by: Peter Chen 
> >Tested-by Joshua Clayton 
> >---
> >  drivers/usb/core/Makefile |   1 +
> >  drivers/usb/core/hub.c|  12 --
> >  drivers/usb/core/hub.h|  12 ++
> >  drivers/usb/core/pwrseq.c | 100 
> > ++
> >  4 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/usb/core/pwrseq.c
> >
> >diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
> >index 9780877..39f2149 100644
> >--- a/drivers/usb/core/Makefile
> >+++ b/drivers/usb/core/Makefile
> >@@ -9,5 +9,6 @@ usbcore-y += port.o of.o
> >  usbcore-$(CONFIG_PCI)  += hcd-pci.o
> >  usbcore-$(CONFIG_ACPI) += usb-acpi.o
> >+usbcore-$(CONFIG_PWRSEQ_GENERIC) += pwrseq.o
> >  obj-$(CONFIG_USB)  += usbcore.o
> >diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >index bee1351..a346a8b 100644
> >--- a/drivers/usb/core/hub.c
> >+++ b/drivers/usb/core/hub.c
> >@@ -1700,6 +1700,7 @@ static void hub_disconnect(struct usb_interface *intf)
> > hub->error = 0;
> > hub_quiesce(hub, HUB_DISCONNECT);
> >+hub_pwrseq_off(hub);
> > mutex_lock(_port_peer_mutex);
> > /* Avoid races with recursively_mark_NOTATTACHED() */
> >@@ -1733,6 +1734,7 @@ static int hub_probe(struct usb_interface *intf, const 
> >struct usb_device_id *id)
> > struct usb_endpoint_descriptor *endpoint;
> > struct usb_device *hdev;
> > struct usb_hub *hub;
> >+int ret = -ENODEV;
> > desc = intf->cur_altsetting;
> > hdev = interface_to_usbdev(intf);
> >@@ -1839,6 +1841,7 @@ descriptor_error:
> > INIT_DELAYED_WORK(>leds, led_work);
> > INIT_DELAYED_WORK(>init_work, NULL);
> > INIT_WORK(>events, hub_event);
> >+INIT_LIST_HEAD(>pwrseq_on_list);
> > usb_get_intf(intf);
> > usb_get_dev(hdev);
> >@@ -1852,11 +1855,14 @@ descriptor_error:
> > if (id->driver_info & HUB_QUIRK_CHECK_PORT_AUTOSUSPEND)
> > hub->quirk_check_port_auto_suspend = 1;
> >-if (hub_configure(hub, endpoint) >= 0)
> >-return 0;
> >+if (hub_configure(hub, endpoint) >= 0) {
> >+ret = hub_pwrseq_on(hub);
> >+if (!ret)
> >+return 0;
> >+}
> > hub_disconnect(intf);
> >-return -ENODEV;
> >+return ret;
> >  }
> >  static int
> >diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> >index 34c1a7e..9473f6f 100644
> >--- a/drivers/usb/core/hub.h
> >+++ b/drivers/usb/core/hub.h
> >@@ -78,6 +78,7 @@ struct usb_hub {
> > struct delayed_work init_work;
> > struct work_struct  events;
> > struct usb_port **ports;
> >+struct list_headpwrseq_on_list; /* powered pwrseq node list */
> >  };
> >  /**
> >@@ -166,3 +167,14 @@ static inline int hub_port_debounce_be_stable(struct 
> >usb_hub *hub,
> >  {
> > return hub_port_debounce(hub, port1, false);
> >  }
> >+
> >+#if IS_ENABLED(CONFIG_PWRSEQ_GENERIC)
> >+int hub_pwrseq_on(struct usb_hub *hub);
> >+void hub_pwrseq_off(struct usb_hub *hub);
> >+#else
> >+static inline int hub_pwrseq_on(struct usb_hub *hub)
> >+{
> >+return 0;
> >+}
> >+static inline void hub_pwrseq_off(struct usb_hub *hub) {}
> >+#endif /* CONFIG_PWRSEQ_GENERIC */
> >diff --git a/drivers/usb/core/pwrseq.c b/drivers/usb/core/pwrseq.c
> >new file mode 100644
> >index 000..837fe66
> >--- /dev/null
> >+++ b/drivers/usb/core/pwrseq.c
> >@@ -0,0 +1,100 @@
> >+/*
> >+ * pwrseq.c USB device power sequence management
> >+ *
> >+ * Copyright (C) 2016 Freescale Semiconductor, Inc.
> >+ * Author: Peter Chen 
> >+ *
> >+ * This program is free software: you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2  of
> >+ * the License as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU 

Re: [PATCH v6 2/8] power: add power sequence library

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:56PM +0530, Vaibhav Hiremath wrote:
> >+static void pwrseq_generic_put(struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk;
> >+
> >+if (pwrseq_gen->gpiod_reset)
> >+gpiod_put(pwrseq_gen->gpiod_reset);
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
> >+clk_put(pwrseq_gen->clks[clk]);
> >+}
> >+
> >+static void pwrseq_generic_off(struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk;
> >+
> >+for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
> >+clk_disable_unprepare(pwrseq_gen->clks[clk]);
> >+}
> >+
> >+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
> Ideally you shouldn't need device_node here at this stage.
> I expect to extract all the resource information in _get() itself.
> 

Agree, I will move reset-duration-us property handling to pwrseq_get.

> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk, ret = 0;
> >+struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
> >+ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
> >+if (ret) {
> >+pr_err("Can't enable clock on %s: %d\n",
> >+np->full_name, ret);
> >+goto err_disable_clks;
> >+}
> >+}
> >+
> >+if (gpiod_reset) {
> >+u32 duration_us = 50;
> Why initialize to 50 ??
> 

This active duration is default value, and should work for most of
devices. If your device needs longer, you can change it at dts.

> >+
> >+of_property_read_u32(np, "reset-duration-us",
> >+_us);
> >+if (duration_us <= 10)
> >+udelay(10);
> >+else
> >+usleep_range(duration_us, duration_us + 100);
> >+gpiod_set_value(gpiod_reset, 0);
> >+}
> >+
> >+return ret;
> >+
> >+err_disable_clks:
> >+while (--clk >= 0)
> >+clk_disable_unprepare(pwrseq_gen->clks[clk]);
> >+
> >+return ret;
> >+}
> >+
> >+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+enum of_gpio_flags flags;
> >+int reset_gpio, clk, ret = 0;
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
> >+pwrseq_gen->clks[clk] = of_clk_get(np, clk);
> >+if (IS_ERR(pwrseq_gen->clks[clk])) {
> >+ret = PTR_ERR(pwrseq_gen->clks[clk]);
> >+if (ret == -EPROBE_DEFER)
> >+goto err_put_clks;
> >+pwrseq_gen->clks[clk] = NULL;
> >+break;
> 
> Do we really want to continue here, even we failed to get the clock ?

Ok, if it is -ENOENT, I set clk as NULL, for other error cases, I go to
error path.

> 
> >+}
> >+}
> >+
> >+reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, );
> >+if (gpio_is_valid(reset_gpio)) {
> >+unsigned long gpio_flags;
> >+
> >+if (flags & OF_GPIO_ACTIVE_LOW)
> >+gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
> >+else
> >+gpio_flags = GPIOF_OUT_INIT_HIGH;
> >+
> >+ret = gpio_request_one(reset_gpio, gpio_flags,
> >+"pwrseq-reset-gpios");
> >+if (ret)
> >+goto err_put_clks;
> >+
> >+pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
> >+} else {
> >+if (reset_gpio == -ENOENT)
> >+return 0;
> 
> Why are we returning success here ?
> 

The property "reset-gpios" is optional, it can be nonexistent.

> >+
> >+ret = reset_gpio;
> >+pr_err("Failed to get reset gpio on %s, err = %d\n",
> >+np->full_name, reset_gpio);
> >+goto err_put_clks;
> >+}
> >+
> >+return ret;
> >+
> >+err_put_clks:
> >+while (--clk >= 0)
> >+clk_put(pwrseq_gen->clks[clk]);
> >+return ret;
> >+}
> >+
> >+struct pwrseq *pwrseq_alloc_generic(void)
> >+{
> >+struct pwrseq_generic *pwrseq_gen;
> >+
> >+pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> >+if (!pwrseq_gen)
> >+return ERR_PTR(-ENOMEM);
> >+
> >+pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> >+pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> >+pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> >+pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> >+pwrseq_gen->pwrseq.free = pwrseq_generic_free;
> >+
> >+return _gen->pwrseq;
> >+}
> >+EXPORT_SYMBOL_GPL(pwrseq_alloc_generic);
> 
> With recent discussion on postcore_initcall, we probably can merge
> _get() and _alloc() 

Re: [PATCH v6 2/8] power: add power sequence library

[Peter Chen]

On Thu, Sep 01, 2016 at 01:32:56PM +0530, Vaibhav Hiremath wrote:
> >+static void pwrseq_generic_put(struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk;
> >+
> >+if (pwrseq_gen->gpiod_reset)
> >+gpiod_put(pwrseq_gen->gpiod_reset);
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++)
> >+clk_put(pwrseq_gen->clks[clk]);
> >+}
> >+
> >+static void pwrseq_generic_off(struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk;
> >+
> >+for (clk = PWRSEQ_MAX_CLKS - 1; clk >= 0; clk--)
> >+clk_disable_unprepare(pwrseq_gen->clks[clk]);
> >+}
> >+
> >+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
> Ideally you shouldn't need device_node here at this stage.
> I expect to extract all the resource information in _get() itself.
> 

Agree, I will move reset-duration-us property handling to pwrseq_get.

> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+int clk, ret = 0;
> >+struct gpio_desc *gpiod_reset = pwrseq_gen->gpiod_reset;
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS && pwrseq_gen->clks[clk]; clk++) {
> >+ret = clk_prepare_enable(pwrseq_gen->clks[clk]);
> >+if (ret) {
> >+pr_err("Can't enable clock on %s: %d\n",
> >+np->full_name, ret);
> >+goto err_disable_clks;
> >+}
> >+}
> >+
> >+if (gpiod_reset) {
> >+u32 duration_us = 50;
> Why initialize to 50 ??
> 

This active duration is default value, and should work for most of
devices. If your device needs longer, you can change it at dts.

> >+
> >+of_property_read_u32(np, "reset-duration-us",
> >+_us);
> >+if (duration_us <= 10)
> >+udelay(10);
> >+else
> >+usleep_range(duration_us, duration_us + 100);
> >+gpiod_set_value(gpiod_reset, 0);
> >+}
> >+
> >+return ret;
> >+
> >+err_disable_clks:
> >+while (--clk >= 0)
> >+clk_disable_unprepare(pwrseq_gen->clks[clk]);
> >+
> >+return ret;
> >+}
> >+
> >+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+enum of_gpio_flags flags;
> >+int reset_gpio, clk, ret = 0;
> >+
> >+for (clk = 0; clk < PWRSEQ_MAX_CLKS; clk++) {
> >+pwrseq_gen->clks[clk] = of_clk_get(np, clk);
> >+if (IS_ERR(pwrseq_gen->clks[clk])) {
> >+ret = PTR_ERR(pwrseq_gen->clks[clk]);
> >+if (ret == -EPROBE_DEFER)
> >+goto err_put_clks;
> >+pwrseq_gen->clks[clk] = NULL;
> >+break;
> 
> Do we really want to continue here, even we failed to get the clock ?

Ok, if it is -ENOENT, I set clk as NULL, for other error cases, I go to
error path.

> 
> >+}
> >+}
> >+
> >+reset_gpio = of_get_named_gpio_flags(np, "reset-gpios", 0, );
> >+if (gpio_is_valid(reset_gpio)) {
> >+unsigned long gpio_flags;
> >+
> >+if (flags & OF_GPIO_ACTIVE_LOW)
> >+gpio_flags = GPIOF_ACTIVE_LOW | GPIOF_OUT_INIT_LOW;
> >+else
> >+gpio_flags = GPIOF_OUT_INIT_HIGH;
> >+
> >+ret = gpio_request_one(reset_gpio, gpio_flags,
> >+"pwrseq-reset-gpios");
> >+if (ret)
> >+goto err_put_clks;
> >+
> >+pwrseq_gen->gpiod_reset = gpio_to_desc(reset_gpio);
> >+} else {
> >+if (reset_gpio == -ENOENT)
> >+return 0;
> 
> Why are we returning success here ?
> 

The property "reset-gpios" is optional, it can be nonexistent.

> >+
> >+ret = reset_gpio;
> >+pr_err("Failed to get reset gpio on %s, err = %d\n",
> >+np->full_name, reset_gpio);
> >+goto err_put_clks;
> >+}
> >+
> >+return ret;
> >+
> >+err_put_clks:
> >+while (--clk >= 0)
> >+clk_put(pwrseq_gen->clks[clk]);
> >+return ret;
> >+}
> >+
> >+struct pwrseq *pwrseq_alloc_generic(void)
> >+{
> >+struct pwrseq_generic *pwrseq_gen;
> >+
> >+pwrseq_gen = kzalloc(sizeof(*pwrseq_gen), GFP_KERNEL);
> >+if (!pwrseq_gen)
> >+return ERR_PTR(-ENOMEM);
> >+
> >+pwrseq_gen->pwrseq.get = pwrseq_generic_get;
> >+pwrseq_gen->pwrseq.on = pwrseq_generic_on;
> >+pwrseq_gen->pwrseq.off = pwrseq_generic_off;
> >+pwrseq_gen->pwrseq.put = pwrseq_generic_put;
> >+pwrseq_gen->pwrseq.free = pwrseq_generic_free;
> >+
> >+return _gen->pwrseq;
> >+}
> >+EXPORT_SYMBOL_GPL(pwrseq_alloc_generic);
> 
> With recent discussion on postcore_initcall, we probably can merge
> _get() and _alloc()