[PATCH] tpm: invalid self test error message
The driver emits invalid self test error message even though the init succeeds. Signed-off-by: Jarkko SakkinenFixes: 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
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
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
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
From: Vivien DidelotDate: 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
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
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
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
From: Emanuel CziraiAMD 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
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
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
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
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
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
> 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
> 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
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
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
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
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
Hi, On Thu, Sep 1, 2016 at 7:35 PM, Ziyuan Xuwrote: > > > 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
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
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
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
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
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
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)
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
Hi, On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiaowrote: > 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
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
Hi, On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiaowrote: > 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
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
Hi, On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiaowrote: > 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
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
Hi, On Thu, Sep 1, 2016 at 8:16 PM, Finley Xiaowrote: > 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
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
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 LiuSigned-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
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
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
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
From: Yakir YangExport 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
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
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
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
From: Yakir YangExport 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
From: Douglas AndersonCurrently 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
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
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
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 XiaoReviewed-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
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 XiaoReviewed-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
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
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
Signed-off-by: Finley XiaoReviewed-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
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
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
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
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
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
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
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
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
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
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
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
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 EllermanSigned-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
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
On 2016年09月02日 05:29, Doug Anderson wrote: Hi, On Wed, Aug 31, 2016 at 11:56 PM, Ziyuan Xuwrote: 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
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
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
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
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
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
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
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
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
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
On Thu, Sep 1, 2016 at 11:31 PM, Maxime Ripardwrote: > 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
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
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
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
Hi, On Thu, Sep 1, 2016 at 11:32 PM, Maxime Ripardwrote: > 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
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
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
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
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
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()