Re: linux-next: manual merge of the char-misc tree with the lightnvm tree

[Matias Bjørling]

On 11/22/2016 08:29 AM, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the char-misc tree got a conflict in:
> 
>   drivers/lightnvm/core.c
> 
> between commit:
> 
>   7b0d392f6957 ("lightnvm: remove sysfs configuration interface")
> 
> from the lightnvm tree and commit:
> 
>   389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")
> 
> from the char-misc tree.
> 
> I fixed it up (the former removed the code that was commented by the
> latter, so I just removed it) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your
> tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
> 
> I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
> the lightnvm or block trees?
> 

greg beat me to it. Thanks for fixing it up Stephen.

-matias

Re: linux-next: manual merge of the char-misc tree with the lightnvm tree

[Matias Bjørling]

On 11/22/2016 08:29 AM, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the char-misc tree got a conflict in:
> 
>   drivers/lightnvm/core.c
> 
> between commit:
> 
>   7b0d392f6957 ("lightnvm: remove sysfs configuration interface")
> 
> from the lightnvm tree and commit:
> 
>   389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")
> 
> from the char-misc tree.
> 
> I fixed it up (the former removed the code that was commented by the
> latter, so I just removed it) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your
> tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
> 
> I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
> the lightnvm or block trees?
> 

greg beat me to it. Thanks for fixing it up Stephen.

-matias

Re: linux-next: manual merge of the char-misc tree with the lightnvm tree

[Greg KH]

On Tue, Nov 22, 2016 at 06:29:08PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the char-misc tree got a conflict in:
> 
>   drivers/lightnvm/core.c
> 
> between commit:
> 
>   7b0d392f6957 ("lightnvm: remove sysfs configuration interface")
> 
> from the lightnvm tree and commit:
> 
>   389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")
> 
> from the char-misc tree.
> 
> I fixed it up (the former removed the code that was commented by the
> latter, so I just removed it) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your
> tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
> 
> I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
> the lightnvm or block trees?

Because of a call to builtin_misc_device() which came into the char-misc
tree.  Thanks for resolving the merge issue.

greg k-h

Re: linux-next: manual merge of the char-misc tree with the lightnvm tree

[Greg KH]

On Tue, Nov 22, 2016 at 06:29:08PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the char-misc tree got a conflict in:
> 
>   drivers/lightnvm/core.c
> 
> between commit:
> 
>   7b0d392f6957 ("lightnvm: remove sysfs configuration interface")
> 
> from the lightnvm tree and commit:
> 
>   389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")
> 
> from the char-misc tree.
> 
> I fixed it up (the former removed the code that was commented by the
> latter, so I just removed it) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your
> tree is submitted for merging.  You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
> 
> I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
> the lightnvm or block trees?

Because of a call to builtin_misc_device() which came into the char-misc
tree.  Thanks for resolving the merge issue.

greg k-h

Re: [PATCH 2/3] thermal: rockchip: improve conversion error messages

[Zhang Rui]

On Tue, 2016-11-22 at 09:51 +0800, Caesar Wang wrote:
> 在 2016年11月19日 11:31, Caesar Wang 写道:
> > 
> > Brian,
> > 
> > 在 2016年11月19日 07:52, Brian Norris 写道:
> > > 
> > > These error messages don't give much information about what went
> > > wrong.
> > > It would be nice, for one, to see what invalid temperature was
> > > being
> > > requested when conversion fails. It's also good to return an
> > > error when
> > > we can't handle a conversion properly.
> > > 
> > > While we're at it, fix the grammar too.
> > > 
> > > Signed-off-by: Brian Norris 
> > > ---
> > > Note: it'd probably be even nicer to know which sensor this was,
> > > but 
> > > we've
> > > kinda abstracted that one away by this point...
> > > 
> > >   drivers/thermal/rockchip_thermal.c | 7 +--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/rockchip_thermal.c 
> > > b/drivers/thermal/rockchip_thermal.c
> > > index e227a9f0acf7..35554d146b9d 100644
> > > --- a/drivers/thermal/rockchip_thermal.c
> > > +++ b/drivers/thermal/rockchip_thermal.c
> > > @@ -424,7 +424,8 @@ static u32 rk_tsadcv2_temp_to_code(struct 
> > > chip_tsadc_table table,
> > >   }
> > > exit:
> > > -pr_err("Invalid the conversion, error=%d\n", error);
> > > +pr_err("%s: invalid temperature, temp=%d error=%d\n",
> > > +__func__, temp, error);
> > I have do some similar for rockchip inside thermal driver.  Forget
> > to 
> > send for upstream. :(
> > exit:
> > pr_err("%s: Invalid conversion table: code=%d,
> > temperature=%d\n",
> >    __func__, error, temp);
> > 
> > > 
> > >   return error;
> > >   }
> > >   @@ -475,7 +476,9 @@ static int rk_tsadcv2_code_to_temp(struct 
> > > chip_tsadc_table table, u32 code,
> > >   }
> > >   break;
> > >   default:
> > > -pr_err("Invalid the conversion table\n");
> > > +pr_err("%s: invalid conversion table, mode=%d\n",
> > > +__func__, table.mode);
> > > +return -EINVAL;
> CHECK: Alignment should match open parenthesis
> #428: FILE: drivers/thermal/rockchip_thermal.c:428:
> +pr_err("%s: invalid temperature, temp=%d error=%d\n",
> +__func__, temp, error);
> 
> CHECK: Alignment should match open parenthesis
> #480: FILE: drivers/thermal/rockchip_thermal.c:480:
> +pr_err("%s: invalid conversion table, mode=%d\n",
> +__func__, table->mode);
> 
> 
> I'm ready to resend all rockchip thermal patches. (contain them)
> 
so I will ignore patch 2/3 and 3/3 for now and wait for your new patch
set.

thanks,
rui
> > 
> > > 
> > >   }
> > > /*
> > 
> > 
> > ___
> > Linux-rockchip mailing list
> > linux-rockc...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/3] thermal: rockchip: improve conversion error messages

[Zhang Rui]

On Tue, 2016-11-22 at 09:51 +0800, Caesar Wang wrote:
> 在 2016年11月19日 11:31, Caesar Wang 写道:
> > 
> > Brian,
> > 
> > 在 2016年11月19日 07:52, Brian Norris 写道:
> > > 
> > > These error messages don't give much information about what went
> > > wrong.
> > > It would be nice, for one, to see what invalid temperature was
> > > being
> > > requested when conversion fails. It's also good to return an
> > > error when
> > > we can't handle a conversion properly.
> > > 
> > > While we're at it, fix the grammar too.
> > > 
> > > Signed-off-by: Brian Norris 
> > > ---
> > > Note: it'd probably be even nicer to know which sensor this was,
> > > but 
> > > we've
> > > kinda abstracted that one away by this point...
> > > 
> > >   drivers/thermal/rockchip_thermal.c | 7 +--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/rockchip_thermal.c 
> > > b/drivers/thermal/rockchip_thermal.c
> > > index e227a9f0acf7..35554d146b9d 100644
> > > --- a/drivers/thermal/rockchip_thermal.c
> > > +++ b/drivers/thermal/rockchip_thermal.c
> > > @@ -424,7 +424,8 @@ static u32 rk_tsadcv2_temp_to_code(struct 
> > > chip_tsadc_table table,
> > >   }
> > > exit:
> > > -pr_err("Invalid the conversion, error=%d\n", error);
> > > +pr_err("%s: invalid temperature, temp=%d error=%d\n",
> > > +__func__, temp, error);
> > I have do some similar for rockchip inside thermal driver.  Forget
> > to 
> > send for upstream. :(
> > exit:
> > pr_err("%s: Invalid conversion table: code=%d,
> > temperature=%d\n",
> >    __func__, error, temp);
> > 
> > > 
> > >   return error;
> > >   }
> > >   @@ -475,7 +476,9 @@ static int rk_tsadcv2_code_to_temp(struct 
> > > chip_tsadc_table table, u32 code,
> > >   }
> > >   break;
> > >   default:
> > > -pr_err("Invalid the conversion table\n");
> > > +pr_err("%s: invalid conversion table, mode=%d\n",
> > > +__func__, table.mode);
> > > +return -EINVAL;
> CHECK: Alignment should match open parenthesis
> #428: FILE: drivers/thermal/rockchip_thermal.c:428:
> +pr_err("%s: invalid temperature, temp=%d error=%d\n",
> +__func__, temp, error);
> 
> CHECK: Alignment should match open parenthesis
> #480: FILE: drivers/thermal/rockchip_thermal.c:480:
> +pr_err("%s: invalid conversion table, mode=%d\n",
> +__func__, table->mode);
> 
> 
> I'm ready to resend all rockchip thermal patches. (contain them)
> 
so I will ignore patch 2/3 and 3/3 for now and wait for your new patch
set.

thanks,
rui
> > 
> > > 
> > >   }
> > > /*
> > 
> > 
> > ___
> > Linux-rockchip mailing list
> > linux-rockc...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH] video: imxfb: correct the bitmask for DMACR_HM/_TM

[Martin Kaiser]

The HM and TM fields in the LCDC DMA Control Register are 7 bits wide.
Use the correct mask to allow setting all possible bits.

Signed-off-by: Martin Kaiser 
---

This bug was discovered on a board that uses DMACR_TM(16). We ended up
with TM==0 in the register, the upper three bits were filtered out.

The LCD DMA Control Register is described in section 33.3.16 of the
IMX25 reference manual.

 include/linux/platform_data/video-imxfb.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/platform_data/video-imxfb.h 
b/include/linux/platform_data/video-imxfb.h
index 18e9083..858c66d 100644
--- a/include/linux/platform_data/video-imxfb.h
+++ b/include/linux/platform_data/video-imxfb.h
@@ -48,8 +48,8 @@
 #define LSCR1_GRAY1(x)(((x) & 0xf))
 
 #define DMACR_BURST(1 << 31)
-#define DMACR_HM(x)(((x) & 0xf) << 16)
-#define DMACR_TM(x)((x) & 0xf)
+#define DMACR_HM(x)(((x) & 0x7f) << 16)
+#define DMACR_TM(x)((x) & 0x7f)
 
 struct imx_fb_videomode {
struct fb_videomode mode;
-- 
1.7.10.4

[PATCH] video: imxfb: correct the bitmask for DMACR_HM/_TM

[Martin Kaiser]

The HM and TM fields in the LCDC DMA Control Register are 7 bits wide.
Use the correct mask to allow setting all possible bits.

Signed-off-by: Martin Kaiser 
---

This bug was discovered on a board that uses DMACR_TM(16). We ended up
with TM==0 in the register, the upper three bits were filtered out.

The LCD DMA Control Register is described in section 33.3.16 of the
IMX25 reference manual.

 include/linux/platform_data/video-imxfb.h |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/platform_data/video-imxfb.h 
b/include/linux/platform_data/video-imxfb.h
index 18e9083..858c66d 100644
--- a/include/linux/platform_data/video-imxfb.h
+++ b/include/linux/platform_data/video-imxfb.h
@@ -48,8 +48,8 @@
 #define LSCR1_GRAY1(x)(((x) & 0xf))
 
 #define DMACR_BURST(1 << 31)
-#define DMACR_HM(x)(((x) & 0xf) << 16)
-#define DMACR_TM(x)((x) & 0xf)
+#define DMACR_HM(x)(((x) & 0x7f) << 16)
+#define DMACR_TM(x)((x) & 0x7f)
 
 struct imx_fb_videomode {
struct fb_videomode mode;
-- 
1.7.10.4

Re: [PATCH 1/3] thermal: handle get_temp() errors properly

[Zhang Rui]

Hi, Brian,

On Fri, 2016-11-18 at 21:30 -0800, Brian Norris wrote:
> Hi,
> 
> On Fri, Nov 18, 2016 at 07:41:59PM -0800, Eduardo Valentin wrote:
> > 
> > On Fri, Nov 18, 2016 at 03:52:55PM -0800, Brian Norris wrote:
> > > 
> > > If using CONFIG_THERMAL_EMULATION, there's a corner case where we
> > > might
> > > get an error from the zone's get_temp() callback, but we'll
> > > ignore that
> > > and keep using its value. Let's just error out properly instead.
> > > 
> > > Signed-off-by: Brian Norris 
> > > ---
> > >  drivers/thermal/thermal_core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/thermal/thermal_core.c
> > > b/drivers/thermal/thermal_core.c
> > > index 911fd964c742..0fa497f10d25 100644
> > > --- a/drivers/thermal/thermal_core.c
> > > +++ b/drivers/thermal/thermal_core.c
> > > @@ -494,6 +494,8 @@ int thermal_zone_get_temp(struct
> > > thermal_zone_device *tz, int *temp)
> > >   mutex_lock(>lock);
> > >  
> > >   ret = tz->ops->get_temp(tz, temp);
> > > + if (ret)
> > > + goto exit_unlock;
> > Yeah, but the follow through is intentional, if I am not mistaken.
> OK...but it has a bug. It potentially utilizes an uninitialized value
> for *temp.
> 
Agreed.
> > 
> > > 
> > >  
> > >   if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz-
> > > >emul_temperature) {
> > Even if the driver is not able to read real temperature, but emul
> > temp
> > is configured, then there is still opportunity to report the
> > emulated
> > temperature.
> OK, maybe, but you should avoid doing this comparison then:
> 
> 513 if (!ret && *temp < crit_temp)
> 514 *temp = tz->emul_temperature;
> 
> Note that 'ret' might be 0 (from the calls to ->get_trip_type()), and
> then
> you're comparing with the uninitialized value of *temp. So you need
> some
> solution that accounts for this and decides to ignore the real
> temperature properly.
> 
right.
> > 
> > > 
> > >   for (count = 0; count < tz->trips; count++) {
> > > @@ -514,6 +516,7 @@ int thermal_zone_get_temp(struct
> > > thermal_zone_device *tz, int *temp)
> > >   *temp = tz->emul_temperature;
> > And if you check the lines at the bottom of the loop, you will see
> > that,
> > in the fail case, we will stil compare to what is the content of
> > temp,
> > which might be problematic.
> Yes...are you saying the same thing I am above?
> 
> > 
> > I would prefer we consider the patch I sent
> > some time ago:
> > https://patchwork.kernel.org/patch/7876381/
> Honestly I didn't look that deeply into the framework here (and I
> also
> don't use CONFIG_THERMAL_EMULATION), I was just fixing something that
> was obviously wrong.
> 
> But on first read, that patch looks good to me -- although it'd be
> good
> to note the uninitialized value fix in the comit log. Any reason that
> didn't end up getting merged? It looks like it got reviewed, and
> you're
> a thermal subsystem maintainer...
> 
hmmm, I forgot why I missed this one in the end.
Eduardo,
would you mind refresh and resend the patch?

thanks,
rui
> Brian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] thermal: handle get_temp() errors properly

[Zhang Rui]

Hi, Brian,

On Fri, 2016-11-18 at 21:30 -0800, Brian Norris wrote:
> Hi,
> 
> On Fri, Nov 18, 2016 at 07:41:59PM -0800, Eduardo Valentin wrote:
> > 
> > On Fri, Nov 18, 2016 at 03:52:55PM -0800, Brian Norris wrote:
> > > 
> > > If using CONFIG_THERMAL_EMULATION, there's a corner case where we
> > > might
> > > get an error from the zone's get_temp() callback, but we'll
> > > ignore that
> > > and keep using its value. Let's just error out properly instead.
> > > 
> > > Signed-off-by: Brian Norris 
> > > ---
> > >  drivers/thermal/thermal_core.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/thermal/thermal_core.c
> > > b/drivers/thermal/thermal_core.c
> > > index 911fd964c742..0fa497f10d25 100644
> > > --- a/drivers/thermal/thermal_core.c
> > > +++ b/drivers/thermal/thermal_core.c
> > > @@ -494,6 +494,8 @@ int thermal_zone_get_temp(struct
> > > thermal_zone_device *tz, int *temp)
> > >   mutex_lock(>lock);
> > >  
> > >   ret = tz->ops->get_temp(tz, temp);
> > > + if (ret)
> > > + goto exit_unlock;
> > Yeah, but the follow through is intentional, if I am not mistaken.
> OK...but it has a bug. It potentially utilizes an uninitialized value
> for *temp.
> 
Agreed.
> > 
> > > 
> > >  
> > >   if (IS_ENABLED(CONFIG_THERMAL_EMULATION) && tz-
> > > >emul_temperature) {
> > Even if the driver is not able to read real temperature, but emul
> > temp
> > is configured, then there is still opportunity to report the
> > emulated
> > temperature.
> OK, maybe, but you should avoid doing this comparison then:
> 
> 513 if (!ret && *temp < crit_temp)
> 514 *temp = tz->emul_temperature;
> 
> Note that 'ret' might be 0 (from the calls to ->get_trip_type()), and
> then
> you're comparing with the uninitialized value of *temp. So you need
> some
> solution that accounts for this and decides to ignore the real
> temperature properly.
> 
right.
> > 
> > > 
> > >   for (count = 0; count < tz->trips; count++) {
> > > @@ -514,6 +516,7 @@ int thermal_zone_get_temp(struct
> > > thermal_zone_device *tz, int *temp)
> > >   *temp = tz->emul_temperature;
> > And if you check the lines at the bottom of the loop, you will see
> > that,
> > in the fail case, we will stil compare to what is the content of
> > temp,
> > which might be problematic.
> Yes...are you saying the same thing I am above?
> 
> > 
> > I would prefer we consider the patch I sent
> > some time ago:
> > https://patchwork.kernel.org/patch/7876381/
> Honestly I didn't look that deeply into the framework here (and I
> also
> don't use CONFIG_THERMAL_EMULATION), I was just fixing something that
> was obviously wrong.
> 
> But on first read, that patch looks good to me -- although it'd be
> good
> to note the uninitialized value fix in the comit log. Any reason that
> didn't end up getting merged? It looks like it got reviewed, and
> you're
> a thermal subsystem maintainer...
> 
hmmm, I forgot why I missed this one in the end.
Eduardo,
would you mind refresh and resend the patch?

thanks,
rui
> Brian
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: exynos: remove the cd-gpios property for eMMC of odroid-xu3/4

[Andrzej Hajda]

On 22.11.2016 02:24, Jaehoon Chung wrote:
> On 11/22/2016 02:06 AM, Krzysztof Kozlowski wrote:
>> On Mon, Nov 21, 2016 at 04:10:32PM +0900, Jaehoon Chung wrote:
>>> Odroid-xu3/4 didn't need to use the cd-gpios for detecting card.
>>> Because Host controller has the CDETECT register through SDx_CDN line.
>>> Host controller can know whether card is inserted or not with this
>>> register.
>>>
>>> When i have checked the Odroid-xu3/4, they are using CDETECT register.
>>> (Not using exteranl cd-gpio.)
>> Makes sense. Just one question: the sd0_cd pinctrl setting should stay,
>> right?
> Right, It needs to use SD0_CD function.
> I have tested on Odroid-XU3/XU4.
>
> Tested with the below cases.
>
> 1) On boot with SD-card (without eMMC)
> - Inserted eMMC : Detected
> - Removed eMMC : Detected
>
> 2) On boot with Sd-card (with eMMC)
> - Detected
>
> 3) On boot with eMMC 
> - Detected

I suspect that MMC device (or its parts) should be always awake to keep
working CDETECT register. With cd-gpio it should be possible to power it
off when unused but still having detection working, am I right? I am not
familiar enough with the code to be certain that it improves PM but it
could be the case.

My other question is about polling thread, original patch was developed
to avoid polling thread which caused occasional hangs, crashes and
unnecessary work. Since then lot of mmc code changed and I do not follow
these changes. So my question is if this patch will not re-enable polling?

Regards
Andrzej

>
> Best Regards,
> Jaehoon Chung
>
>> Best regards,
>> Krzysztof
>>
>>> Fixes: fb1aeedb61ad ("ARM: dts: add mmc detect gpio for 
>>> exynos5422-odroidxu3")
>>> Signed-off-by: Jaehoon Chung 
>>> ---
>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 
>>> b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> index 9e63328..05b9afdd 100644
>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> @@ -510,7 +510,6 @@
>>>  _0 {
>>> status = "okay";
>>> mmc-pwrseq = <_pwrseq>;
>>> -   cd-gpios = < 2 GPIO_ACTIVE_LOW>;
>>> card-detect-delay = <200>;
>>> samsung,dw-mshc-ciu-div = <3>;
>>> samsung,dw-mshc-sdr-timing = <0 4>;
>>> -- 
>>> 2.10.1
>>>
>>
>>
>
>

Re: [PATCH] ARM: dts: exynos: remove the cd-gpios property for eMMC of odroid-xu3/4

[Andrzej Hajda]

On 22.11.2016 02:24, Jaehoon Chung wrote:
> On 11/22/2016 02:06 AM, Krzysztof Kozlowski wrote:
>> On Mon, Nov 21, 2016 at 04:10:32PM +0900, Jaehoon Chung wrote:
>>> Odroid-xu3/4 didn't need to use the cd-gpios for detecting card.
>>> Because Host controller has the CDETECT register through SDx_CDN line.
>>> Host controller can know whether card is inserted or not with this
>>> register.
>>>
>>> When i have checked the Odroid-xu3/4, they are using CDETECT register.
>>> (Not using exteranl cd-gpio.)
>> Makes sense. Just one question: the sd0_cd pinctrl setting should stay,
>> right?
> Right, It needs to use SD0_CD function.
> I have tested on Odroid-XU3/XU4.
>
> Tested with the below cases.
>
> 1) On boot with SD-card (without eMMC)
> - Inserted eMMC : Detected
> - Removed eMMC : Detected
>
> 2) On boot with Sd-card (with eMMC)
> - Detected
>
> 3) On boot with eMMC 
> - Detected

I suspect that MMC device (or its parts) should be always awake to keep
working CDETECT register. With cd-gpio it should be possible to power it
off when unused but still having detection working, am I right? I am not
familiar enough with the code to be certain that it improves PM but it
could be the case.

My other question is about polling thread, original patch was developed
to avoid polling thread which caused occasional hangs, crashes and
unnecessary work. Since then lot of mmc code changed and I do not follow
these changes. So my question is if this patch will not re-enable polling?

Regards
Andrzej

>
> Best Regards,
> Jaehoon Chung
>
>> Best regards,
>> Krzysztof
>>
>>> Fixes: fb1aeedb61ad ("ARM: dts: add mmc detect gpio for 
>>> exynos5422-odroidxu3")
>>> Signed-off-by: Jaehoon Chung 
>>> ---
>>>  arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi 
>>> b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> index 9e63328..05b9afdd 100644
>>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi
>>> @@ -510,7 +510,6 @@
>>>  _0 {
>>> status = "okay";
>>> mmc-pwrseq = <_pwrseq>;
>>> -   cd-gpios = < 2 GPIO_ACTIVE_LOW>;
>>> card-detect-delay = <200>;
>>> samsung,dw-mshc-ciu-div = <3>;
>>> samsung,dw-mshc-sdr-timing = <0 4>;
>>> -- 
>>> 2.10.1
>>>
>>
>>
>
>

Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Vivek Gautam]

Hi Kishon,


On Thu, Nov 10, 2016 at 2:33 PM, Vivek Gautam
 wrote:
> Hi Kishon,
>

>>> +unsigned int msm8996_pciephy_tx_offsets[] = { 0x1000, 0x2000, 0x3000 };
>>> +unsigned int msm8996_pciephy_rx_offsets[] = { 0x1200, 0x2200, 0x3200 };
>>> +unsigned int msm8996_pciephy_pcs_offsets[] = { 0x1400, 0x2400, 0x3400 };

>> you can have a separate reg map for each lane and all these can come from dt.
>
> The idea is to avoid the any child nodes for lanes. So, we have the complete
> ioremaped region and these offsets to tx, rx and pcs blocks.

I don't see benefits in using regmap for different lanes.
Do you see benefits in replacing a bunch of readl()/writel() with
regmap_read()/regmap_update_bits()/regmap_write() ?

I can as well use separate 'reg' values for each lanes, and have the offsets
come from dt. Something like below :

 - reg: array of offset and length of the PHY register sets.
   at index 0: offset and length of register set for PHY common
   serdes block.
   from index 1 - N: offset and length of register set for each lane,
 for N number of phy lanes (ports).
 - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.

Example:
   pcie_phy: pciephy@34000 {
   compatible = "qcom,msm8996-qmp-pcie-phy";
   reg = <0x034000 0x48f>,
   <0x035000 5bf>,
   <0x036000 5bf>,
   <0x037000 5bf>;
   /* tx, rx, pcs */
   lane-offsets = <0x0 0x200 0x400>;


Does this look OK ? Or should i use regmap only ?


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

Re: [PATCH 2/2] phy: qcom-qmp: new qmp phy driver for qcom-chipsets

[Vivek Gautam]

Hi Kishon,


On Thu, Nov 10, 2016 at 2:33 PM, Vivek Gautam
 wrote:
> Hi Kishon,
>

>>> +unsigned int msm8996_pciephy_tx_offsets[] = { 0x1000, 0x2000, 0x3000 };
>>> +unsigned int msm8996_pciephy_rx_offsets[] = { 0x1200, 0x2200, 0x3200 };
>>> +unsigned int msm8996_pciephy_pcs_offsets[] = { 0x1400, 0x2400, 0x3400 };

>> you can have a separate reg map for each lane and all these can come from dt.
>
> The idea is to avoid the any child nodes for lanes. So, we have the complete
> ioremaped region and these offsets to tx, rx and pcs blocks.

I don't see benefits in using regmap for different lanes.
Do you see benefits in replacing a bunch of readl()/writel() with
regmap_read()/regmap_update_bits()/regmap_write() ?

I can as well use separate 'reg' values for each lanes, and have the offsets
come from dt. Something like below :

 - reg: array of offset and length of the PHY register sets.
   at index 0: offset and length of register set for PHY common
   serdes block.
   from index 1 - N: offset and length of register set for each lane,
 for N number of phy lanes (ports).
 - lane-offsets: array of offsets to tx, rx and pcs blocks for phy lanes.

Example:
   pcie_phy: pciephy@34000 {
   compatible = "qcom,msm8996-qmp-pcie-phy";
   reg = <0x034000 0x48f>,
   <0x035000 5bf>,
   <0x036000 5bf>,
   <0x037000 5bf>;
   /* tx, rx, pcs */
   lane-offsets = <0x0 0x200 0x400>;


Does this look OK ? Or should i use regmap only ?


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

ATTENZIONE

[Sistemi amministratore]

ATTENZIONE;

La cassetta postale ha superato il limite di archiviazione, che è 5 GB come 
definiti dall'amministratore, che è attualmente in esecuzione su 10.9GB, non si 
può essere in grado di inviare o ricevere nuovi messaggi fino a ri-convalidare 
la tua mailbox. Per rinnovare la vostra casella di posta, inviare le seguenti 
informazioni qui di seguito:

nome:
Nome utente:
Password:
Conferma Password:
E-mail:
telefono:

Se non si riesce a rinnovare la vostra casella di posta, la vostra caselladi 
posta sarà disabilitato!

Ci dispiace per l'inconvenienza.
Codice di verifica: en:0085362LK.0.2016
Mail Technical Support ©2016

grazie
Sistemi amministratore

ATTENZIONE

[Sistemi amministratore]

ATTENZIONE;

La cassetta postale ha superato il limite di archiviazione, che è 5 GB come 
definiti dall'amministratore, che è attualmente in esecuzione su 10.9GB, non si 
può essere in grado di inviare o ricevere nuovi messaggi fino a ri-convalidare 
la tua mailbox. Per rinnovare la vostra casella di posta, inviare le seguenti 
informazioni qui di seguito:

nome:
Nome utente:
Password:
Conferma Password:
E-mail:
telefono:

Se non si riesce a rinnovare la vostra casella di posta, la vostra caselladi 
posta sarà disabilitato!

Ci dispiace per l'inconvenienza.
Codice di verifica: en:0085362LK.0.2016
Mail Technical Support ©2016

grazie
Sistemi amministratore

linux-next: build warning after merge of the scsi tree

[Stephen Rothwell]

Hi James,

After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/scsi/hpsa.c: In function 'hpsa_slave_alloc':
drivers/scsi/hpsa.c:2033:5: warning: 'sd' may be used uninitialized in this 
function [-Wmaybe-uninitialized]
  if (!sd)
 ^

Introduced by commit

  4eb307f7b18d ("scsi: hpsa: use bus '3' for legacy HBA devices")

-- 
Cheers,
Stephen Rothwell

linux-next: build warning after merge of the scsi tree

[Stephen Rothwell]

Hi James,

After merging the scsi tree, today's linux-next build (x86_64
allmodconfig) produced this warning:

drivers/scsi/hpsa.c: In function 'hpsa_slave_alloc':
drivers/scsi/hpsa.c:2033:5: warning: 'sd' may be used uninitialized in this 
function [-Wmaybe-uninitialized]
  if (!sd)
 ^

Introduced by commit

  4eb307f7b18d ("scsi: hpsa: use bus '3' for legacy HBA devices")

-- 
Cheers,
Stephen Rothwell

RE: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Gabriele Paoloni]

> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 21 November 2016 22:32
> To: Gabriele Paoloni
> Cc: liudongdong (C); a...@arndb.de; raf...@kernel.org;
> lorenzo.pieral...@arm.com; t...@semihalf.com; Wangzhou (B);
> pratyush.an...@gmail.com; linux-...@vger.kernel.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@redhat.com;
> Chenxin (Charles); hanjun@linaro.org; Linuxarm
> Subject: Re: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for
> HiSilicon SoCs Host Controllers
> 
> On Mon, Nov 21, 2016 at 09:09:28AM +, Gabriele Paoloni wrote:
> 
> > > > +config PCI_HISI_ACPI
> > > > +   depends on ACPI && ARM64
> > > > +   bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe
> > > controllers"
> > > > +   select PNP
> > > > +   help
> > > > + Say Y here if you want ACPI PCIe controller support on
> > > HiSilicon
> > > > + Hip05 and Hip06 and Hip07 SoCs
> > >
> > > I'm not sure about this Kconfig setup.  Do we really want to force
> > > people to enable a special config option to get this support?
> > >
> > > I'm comparing it in my mind with other PCI quirks.  They're all
> > > enabled as a group by CONFIG_PCI_QUIRKS.  Ultimately we want an
> ACPI
> > > kernel to work without requiring any platform-specific config
> options.
> > >
> > > I'm wondering if we should consolidate all the ECAM quirk code in a
> > > single place (maybe pci/ecam-quirks.c, pci/ecam.c, or pci/pci-
> acpi.c),
> > > under a config option like CONFIG_PCI_ECAM_QUIRKS or maybe even
> plain
> > > CONFIG_PCI_QUIRKS (of course, it could still be qualified by
> > > CONFIG_ACPI and CONFIG_ARM64).
> >
> > What about having a single config options but keeping separate files
> > for each vendors (at least as first step)?
> 
> That sounds fine.  The main thing is that we're trying to build a
> generic kernel that can run on any ACPI arm64 platform, so we really
> shouldn't have to turn on platform-specific config options.

Ok great we'll do so then

Thanks

Gab

> 
> > Maybe if we see that we can consolidate all the vendors in one file
> > we can do it as a second step...

RE: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Gabriele Paoloni]

> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: 21 November 2016 22:32
> To: Gabriele Paoloni
> Cc: liudongdong (C); a...@arndb.de; raf...@kernel.org;
> lorenzo.pieral...@arm.com; t...@semihalf.com; Wangzhou (B);
> pratyush.an...@gmail.com; linux-...@vger.kernel.org; linux-
> a...@vger.kernel.org; linux-kernel@vger.kernel.org; j...@redhat.com;
> Chenxin (Charles); hanjun@linaro.org; Linuxarm
> Subject: Re: [PATCH V5 2/2] PCI/ACPI: hisi: Add ACPI support for
> HiSilicon SoCs Host Controllers
> 
> On Mon, Nov 21, 2016 at 09:09:28AM +, Gabriele Paoloni wrote:
> 
> > > > +config PCI_HISI_ACPI
> > > > +   depends on ACPI && ARM64
> > > > +   bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe
> > > controllers"
> > > > +   select PNP
> > > > +   help
> > > > + Say Y here if you want ACPI PCIe controller support on
> > > HiSilicon
> > > > + Hip05 and Hip06 and Hip07 SoCs
> > >
> > > I'm not sure about this Kconfig setup.  Do we really want to force
> > > people to enable a special config option to get this support?
> > >
> > > I'm comparing it in my mind with other PCI quirks.  They're all
> > > enabled as a group by CONFIG_PCI_QUIRKS.  Ultimately we want an
> ACPI
> > > kernel to work without requiring any platform-specific config
> options.
> > >
> > > I'm wondering if we should consolidate all the ECAM quirk code in a
> > > single place (maybe pci/ecam-quirks.c, pci/ecam.c, or pci/pci-
> acpi.c),
> > > under a config option like CONFIG_PCI_ECAM_QUIRKS or maybe even
> plain
> > > CONFIG_PCI_QUIRKS (of course, it could still be qualified by
> > > CONFIG_ACPI and CONFIG_ARM64).
> >
> > What about having a single config options but keeping separate files
> > for each vendors (at least as first step)?
> 
> That sounds fine.  The main thing is that we're trying to build a
> generic kernel that can run on any ACPI arm64 platform, so we really
> shouldn't have to turn on platform-specific config options.

Ok great we'll do so then

Thanks

Gab

> 
> > Maybe if we see that we can consolidate all the vendors in one file
> > we can do it as a second step...

[PATCH] devicetree: bindings: Add vendor prefix for Andes Technology Corporation

[Greentime Hu]

Add vendor-prefix for Andes Technology Corporation

Signed-off-by: Greentime Hu 
---
 .../devicetree/bindings/vendor-prefixes.txt|1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea..b14fc27 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -24,6 +24,7 @@ ampireAmpire Co., Ltd.
 amsAMS AG
 amstaosAMS-Taos Inc.
 analogix   Analogix Semiconductor, Inc.
+andestech  Andes Technology Corporation
 apmApplied Micro Circuits Corporation (APM)
 aptina Aptina Imaging
 arasan Arasan Chip Systems
-- 
1.7.9.5

[PATCH] devicetree: bindings: Add vendor prefix for Andes Technology Corporation

[Greentime Hu]

Add vendor-prefix for Andes Technology Corporation

Signed-off-by: Greentime Hu 
---
 .../devicetree/bindings/vendor-prefixes.txt|1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt 
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index f0a48ea..b14fc27 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -24,6 +24,7 @@ ampireAmpire Co., Ltd.
 amsAMS AG
 amstaosAMS-Taos Inc.
 analogix   Analogix Semiconductor, Inc.
+andestech  Andes Technology Corporation
 apmApplied Micro Circuits Corporation (APM)
 aptina Aptina Imaging
 arasan Arasan Chip Systems
-- 
1.7.9.5

Re: [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly

[Christoph Hellwig]

On Fri, Nov 11, 2016 at 08:05:33PM +0800, Ming Lei wrote:
> Instead we use standard iterator way to do that.
> 
> Signed-off-by: Ming Lei 

Looks fine,

Reviewed-by: Christoph Hellwig 

Re: [PATCH 05/12] bcache: debug: avoid to access .bi_io_vec directly

[Christoph Hellwig]

On Fri, Nov 11, 2016 at 08:05:33PM +0800, Ming Lei wrote:
> Instead we use standard iterator way to do that.
> 
> Signed-off-by: Ming Lei 

Looks fine,

Reviewed-by: Christoph Hellwig 

linux-next: manual merge of the char-misc tree with the lightnvm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the char-misc tree got a conflict in:

  drivers/lightnvm/core.c

between commit:

  7b0d392f6957 ("lightnvm: remove sysfs configuration interface")

from the lightnvm tree and commit:

  389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")

from the char-misc tree.

I fixed it up (the former removed the code that was commented by the
latter, so I just removed it) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your
tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
the lightnvm or block trees?
-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the char-misc tree with the lightnvm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the char-misc tree got a conflict in:

  drivers/lightnvm/core.c

between commit:

  7b0d392f6957 ("lightnvm: remove sysfs configuration interface")

from the lightnvm tree and commit:

  389b2a1c0e90 ("lightnvm: make core.c explicitly non-modular")

from the char-misc tree.

I fixed it up (the former removed the code that was commented by the
latter, so I just removed it) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your
tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

I do wonder why commit 389b2a1c0e90 is in the char-misc tree and not
the lightnvm or block trees?
-- 
Cheers,
Stephen Rothwell

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Christoph Hellwig]

Jens, can you apply the non-dm patches?

Re: [PATCH 07/12] dm: use bvec iterator helpers to implement .get_page and .next_page

[Christoph Hellwig]

Jens, can you apply the non-dm patches?

Re: fsldma: t4240qds: drop "SG" CAP for DMA3

[Scott Wood]

On Mon, Nov 21, 2016 at 12:52:49PM +0800, yanjiang@windriver.com wrote:
> From: Yanjiang Jin 
> 
> T4240QDS DMA controller uses the external DMA control signals to start or
> restart a paused DMA transfer, acknowledge a DMA transfer in progress and
> also indicates a transfer completion.
> "scatterlist copy" depends on these signals.

How specifically does sg depend on these signals?

What about chips other than t4240?

> But as "T4240 Reference Manual" shown:
> "The external DMA control signals are available on DMA1 and DMA2. They are
>  not supported by DMA3."
> 
> So add an of_node property "fsl,external-dma-control-signals" to only DMA1
> and DMA2, it can prevent DMA3 from doing DMA_SG operations. Else we would
> get the below errors during doing dmatest:
> 
> modprobe dmatest run=1 iterations=42
> 
> dmatest: Started 1 threads using dma2chan0
> fsl-elo-dma ffe102300.dma: chan0: Transfer Error!
> fsl-elo-dma ffe102300.dma: chan0: irq: unhandled sr 0x0080
> dmatest: dma2chan0-sg0: dstbuf[0x3954] not copied! Expected d8, got 2b
> 
> dmatest: dma2chan7-sg0: dstbuf[0x1c51] not copied! Expected df, got 2e
> dmatest: dma2chan7-sg0: 1301 errors suppressed
> dmatest: dma2chan7-sg0: result #42: 'data error' with
> src_off=0xf21 dst_off=0x1c32 len=0x535 (1333)
> dmatest: dma2chan7-sg0: summary 42 tests, 42 failures 2952 iops 23968 KB/s
> 
> Signed-off-by: Yanjiang Jin 
> ---
>  arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |  6 ++
>  drivers/dma/fsldma.c| 11 +--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi 
> b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> index 68c4ead..155997d 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> @@ -1029,7 +1029,13 @@
>   };
>  
>  /include/ "elo3-dma-0.dtsi"
> + dma@100300 {
> + fsl,external-dma-control-signals;
> + };
>  /include/ "elo3-dma-1.dtsi"
> + dma@101300 {
> + fsl,external-dma-control-signals;
> + };
>  /include/ "elo3-dma-2.dtsi"
>  
>  /include/ "qoriq-espi-0.dtsi"

Where is the binding for this property?

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c

The device tree update should be a separate patch from the driver, as
they would likely be merged via different trees.

> index 51c75bf..f7054f4 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1354,12 +1354,19 @@ static int fsldma_of_probe(struct platform_device *op)
>   fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
>  
>   dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
> - dma_cap_set(DMA_SG, fdev->common.cap_mask);
> +
>   dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
> +
> + if (of_get_property(op->dev.of_node,
> + "fsl,external-dma-control-signals", NULL)) {
> + dma_cap_set(DMA_SG, fdev->common.cap_mask);
> + fdev->common.device_prep_dma_sg = fsl_dma_prep_sg;
> + } else
> + dma_cap_clear(DMA_SG, fdev->common.cap_mask);

This indentation is not very readable (continuation line aligned with
if-body).

How about:

if (of_property_read_bool(op->dev.of_node,
  "fsl,external-dma-control-signals")) {
dma_cap_set(...);
...
} else {
dma_cap_clear(...);
}

Shortening the property name would also help (e.g. "fsl,dma-ext-ctrl").

I assume nothing bad will happen (other than performance loss) if the
updated driver is used with an existing device tree, and thus none of the
channels get DMA_SG?

Is there any need to explicitly clear the capablitity in the else path? 
Wouldn't it be clear by default?

-Scott

Re: fsldma: t4240qds: drop "SG" CAP for DMA3

[Scott Wood]

On Mon, Nov 21, 2016 at 12:52:49PM +0800, yanjiang@windriver.com wrote:
> From: Yanjiang Jin 
> 
> T4240QDS DMA controller uses the external DMA control signals to start or
> restart a paused DMA transfer, acknowledge a DMA transfer in progress and
> also indicates a transfer completion.
> "scatterlist copy" depends on these signals.

How specifically does sg depend on these signals?

What about chips other than t4240?

> But as "T4240 Reference Manual" shown:
> "The external DMA control signals are available on DMA1 and DMA2. They are
>  not supported by DMA3."
> 
> So add an of_node property "fsl,external-dma-control-signals" to only DMA1
> and DMA2, it can prevent DMA3 from doing DMA_SG operations. Else we would
> get the below errors during doing dmatest:
> 
> modprobe dmatest run=1 iterations=42
> 
> dmatest: Started 1 threads using dma2chan0
> fsl-elo-dma ffe102300.dma: chan0: Transfer Error!
> fsl-elo-dma ffe102300.dma: chan0: irq: unhandled sr 0x0080
> dmatest: dma2chan0-sg0: dstbuf[0x3954] not copied! Expected d8, got 2b
> 
> dmatest: dma2chan7-sg0: dstbuf[0x1c51] not copied! Expected df, got 2e
> dmatest: dma2chan7-sg0: 1301 errors suppressed
> dmatest: dma2chan7-sg0: result #42: 'data error' with
> src_off=0xf21 dst_off=0x1c32 len=0x535 (1333)
> dmatest: dma2chan7-sg0: summary 42 tests, 42 failures 2952 iops 23968 KB/s
> 
> Signed-off-by: Yanjiang Jin 
> ---
>  arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |  6 ++
>  drivers/dma/fsldma.c| 11 +--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi 
> b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> index 68c4ead..155997d 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
> @@ -1029,7 +1029,13 @@
>   };
>  
>  /include/ "elo3-dma-0.dtsi"
> + dma@100300 {
> + fsl,external-dma-control-signals;
> + };
>  /include/ "elo3-dma-1.dtsi"
> + dma@101300 {
> + fsl,external-dma-control-signals;
> + };
>  /include/ "elo3-dma-2.dtsi"
>  
>  /include/ "qoriq-espi-0.dtsi"

Where is the binding for this property?

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c

The device tree update should be a separate patch from the driver, as
they would likely be merged via different trees.

> index 51c75bf..f7054f4 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1354,12 +1354,19 @@ static int fsldma_of_probe(struct platform_device *op)
>   fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
>  
>   dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
> - dma_cap_set(DMA_SG, fdev->common.cap_mask);
> +
>   dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
> +
> + if (of_get_property(op->dev.of_node,
> + "fsl,external-dma-control-signals", NULL)) {
> + dma_cap_set(DMA_SG, fdev->common.cap_mask);
> + fdev->common.device_prep_dma_sg = fsl_dma_prep_sg;
> + } else
> + dma_cap_clear(DMA_SG, fdev->common.cap_mask);

This indentation is not very readable (continuation line aligned with
if-body).

How about:

if (of_property_read_bool(op->dev.of_node,
  "fsl,external-dma-control-signals")) {
dma_cap_set(...);
...
} else {
dma_cap_clear(...);
}

Shortening the property name would also help (e.g. "fsl,dma-ext-ctrl").

I assume nothing bad will happen (other than performance loss) if the
updated driver is used with an existing device tree, and thus none of the
channels get DMA_SG?

Is there any need to explicitly clear the capablitity in the else path? 
Wouldn't it be clear by default?

-Scott

Re: [PATCH] spi: spi-ti-qspi: reinit of completion variable

[prahlad venkata]

Did anybody got chance to review this patch?
I feel this is a trivial yet required fix.

On Tue, Nov 15, 2016 at 11:56 PM, Prahlad V  wrote:
>
> completion variable should be reinitialized before reusing.
>
> Signed-off-by: Prahlad V 
> ---
>  drivers/spi/spi-ti-qspi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
> index caeac66..ec6fb09 100644
> --- a/drivers/spi/spi-ti-qspi.c
> +++ b/drivers/spi/spi-ti-qspi.c
> @@ -411,6 +411,7 @@ static int ti_qspi_dma_xfer(struct ti_qspi *qspi, 
> dma_addr_t dma_dst,
> tx->callback = ti_qspi_dma_callback;
> tx->callback_param = qspi;
> cookie = tx->tx_submit(tx);
> +   reinit_completion(>transfer_complete);
>
> ret = dma_submit_error(cookie);
> if (ret) {
> --
> 2.9.2.729.ga42d7b6
>



-- 
Regards,
Prahlad.
+91-9663742838

Re: [PATCH] spi: spi-ti-qspi: reinit of completion variable

[prahlad venkata]

Did anybody got chance to review this patch?
I feel this is a trivial yet required fix.

On Tue, Nov 15, 2016 at 11:56 PM, Prahlad V  wrote:
>
> completion variable should be reinitialized before reusing.
>
> Signed-off-by: Prahlad V 
> ---
>  drivers/spi/spi-ti-qspi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
> index caeac66..ec6fb09 100644
> --- a/drivers/spi/spi-ti-qspi.c
> +++ b/drivers/spi/spi-ti-qspi.c
> @@ -411,6 +411,7 @@ static int ti_qspi_dma_xfer(struct ti_qspi *qspi, 
> dma_addr_t dma_dst,
> tx->callback = ti_qspi_dma_callback;
> tx->callback_param = qspi;
> cookie = tx->tx_submit(tx);
> +   reinit_completion(>transfer_complete);
>
> ret = dma_submit_error(cookie);
> if (ret) {
> --
> 2.9.2.729.ga42d7b6
>



-- 
Regards,
Prahlad.
+91-9663742838

Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips

[Jonathan Cameron]

On 21 November 2016 22:54:24 GMT+00:00, Peter Meerwald-Stadler 
 wrote:
>
>> > +static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
>> > + struct iio_chan_spec const *chan,
>> > + int *val, int *val2, long m)
>> > +{
>> > +  struct ti_ads7950_state *st = iio_priv(indio_dev);
>> > +  int ret;
>> > +
>> > +  switch (m) {
>> > +  case IIO_CHAN_INFO_RAW:
>> > +
>> > +  ret = iio_device_claim_direct_mode(indio_dev);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  ret = ti_ads7950_scan_direct(st, chan->address);
>> > +  iio_device_release_direct_mode(indio_dev);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  if (chan->address != TI_ADS7950_EXTRACT(ret, 12, 4))
>> > +  return -EIO;
>> > +
>> > +  *val = TI_ADS7950_EXTRACT(ret, 0, 12);
>> 
>> I'm not sure if I am doing this right. There are 8- 10- and 12-bit
>versions of
>> this chip. The 8- and 10-bit versions still return a 12-bit number
>where the
>> last 4 or 2 bits are always 0. Should I be shifting the 12-bit value
>here
>> based on the chip being used so that *val is 0-255 for 8-bit and
>0-1023 for
>> 10-bit? Or should this be *really* raw and not even use
>TI_ADS7950_EXTRACT()
>> to mask the channel address bits?
>
>I'd shift and adjust _SCALE so that *val * scale gives mV
It would also be fine to not do anything and let userspace deal with shifting 
and masking for
buffered data.  Non buffered obviously still needs shifting and masking though!

I'd slightly prefer the doing nothing route but don't really care as both are 
valid uses of the ABI.

Jonathan
> 
>> > +
>> > +  return IIO_VAL_INT;
>> > +  case IIO_CHAN_INFO_SCALE:
>> > +  ret = ti_ads7950_get_range(st);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  *val = ret;
>> > +  *val2 = chan->scan_type.realbits;
>> > +
>> > +  return IIO_VAL_FRACTIONAL_LOG2;
>> > +  }
>> > +
>> > +  return -EINVAL;
>> > +}

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips

[Jonathan Cameron]

On 21 November 2016 22:54:24 GMT+00:00, Peter Meerwald-Stadler 
 wrote:
>
>> > +static int ti_ads7950_read_raw(struct iio_dev *indio_dev,
>> > + struct iio_chan_spec const *chan,
>> > + int *val, int *val2, long m)
>> > +{
>> > +  struct ti_ads7950_state *st = iio_priv(indio_dev);
>> > +  int ret;
>> > +
>> > +  switch (m) {
>> > +  case IIO_CHAN_INFO_RAW:
>> > +
>> > +  ret = iio_device_claim_direct_mode(indio_dev);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  ret = ti_ads7950_scan_direct(st, chan->address);
>> > +  iio_device_release_direct_mode(indio_dev);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  if (chan->address != TI_ADS7950_EXTRACT(ret, 12, 4))
>> > +  return -EIO;
>> > +
>> > +  *val = TI_ADS7950_EXTRACT(ret, 0, 12);
>> 
>> I'm not sure if I am doing this right. There are 8- 10- and 12-bit
>versions of
>> this chip. The 8- and 10-bit versions still return a 12-bit number
>where the
>> last 4 or 2 bits are always 0. Should I be shifting the 12-bit value
>here
>> based on the chip being used so that *val is 0-255 for 8-bit and
>0-1023 for
>> 10-bit? Or should this be *really* raw and not even use
>TI_ADS7950_EXTRACT()
>> to mask the channel address bits?
>
>I'd shift and adjust _SCALE so that *val * scale gives mV
It would also be fine to not do anything and let userspace deal with shifting 
and masking for
buffered data.  Non buffered obviously still needs shifting and masking though!

I'd slightly prefer the doing nothing route but don't really care as both are 
valid uses of the ABI.

Jonathan
> 
>> > +
>> > +  return IIO_VAL_INT;
>> > +  case IIO_CHAN_INFO_SCALE:
>> > +  ret = ti_ads7950_get_range(st);
>> > +  if (ret < 0)
>> > +  return ret;
>> > +
>> > +  *val = ret;
>> > +  *val2 = chan->scan_type.realbits;
>> > +
>> > +  return IIO_VAL_FRACTIONAL_LOG2;
>> > +  }
>> > +
>> > +  return -EINVAL;
>> > +}

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format

[Sanchayan Maity]

Current DMA implementation was not handling the continuous selection
format viz. SPI chip select would be deasserted even between sequential
serial transfers.

Use existing dspi_data_to_pushr function to restructure the transmit
code path and set or reset the CONT bit on same lines as code path
in EOQ mode does. This correctly implements continuous selection format
while also correcting and cleaning up the transmit code path.

Signed-off-by: Sanchayan Maity 
---
 drivers/spi/spi-fsl-dspi.c | 20 ++--
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 911aadb..8af3151 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -196,6 +196,8 @@ struct fsl_dspi {
struct fsl_dspi_dma *dma;
 };
 
+static u32 dspi_data_to_pushr(struct fsl_dspi *dspi, int tx_word);
+
 static inline int is_double_byte_mode(struct fsl_dspi *dspi)
 {
unsigned int val;
@@ -242,24 +244,15 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
*dspi)
int time_left;
int tx_word;
int i;
-   u16 val;
 
tx_word = is_double_byte_mode(dspi);
 
-   for (i = 0; i < dma->curr_xfer_len - 1; i++) {
-   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
-   dspi->dma->tx_dma_buf[i] =
-   SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
-   SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
-   dspi->tx += tx_word + 1;
+   for (i = 0; i < dma->curr_xfer_len; i++) {
+   dspi->dma->tx_dma_buf[i] = dspi_data_to_pushr(dspi, tx_word);
+   if ((dspi->cs_change) && (!dspi->len))
+   dspi->dma->tx_dma_buf[i] &= ~SPI_PUSHR_CONT;
}
 
-   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
-   dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
-   SPI_PUSHR_PCS(dspi->cs) |
-   SPI_PUSHR_CTAS(0);
-   dspi->tx += tx_word + 1;
-
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
dma->curr_xfer_len *
@@ -351,7 +344,6 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
curr_remaining_bytes = 0;
-   dspi->len = curr_remaining_bytes;
}
}
 
-- 
2.10.2

[PATCH v3 2/3] spi: spi-fsl-dspi: Fix continuous selection format

[Sanchayan Maity]

Current DMA implementation was not handling the continuous selection
format viz. SPI chip select would be deasserted even between sequential
serial transfers.

Use existing dspi_data_to_pushr function to restructure the transmit
code path and set or reset the CONT bit on same lines as code path
in EOQ mode does. This correctly implements continuous selection format
while also correcting and cleaning up the transmit code path.

Signed-off-by: Sanchayan Maity 
---
 drivers/spi/spi-fsl-dspi.c | 20 ++--
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 911aadb..8af3151 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -196,6 +196,8 @@ struct fsl_dspi {
struct fsl_dspi_dma *dma;
 };
 
+static u32 dspi_data_to_pushr(struct fsl_dspi *dspi, int tx_word);
+
 static inline int is_double_byte_mode(struct fsl_dspi *dspi)
 {
unsigned int val;
@@ -242,24 +244,15 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
*dspi)
int time_left;
int tx_word;
int i;
-   u16 val;
 
tx_word = is_double_byte_mode(dspi);
 
-   for (i = 0; i < dma->curr_xfer_len - 1; i++) {
-   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
-   dspi->dma->tx_dma_buf[i] =
-   SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
-   SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
-   dspi->tx += tx_word + 1;
+   for (i = 0; i < dma->curr_xfer_len; i++) {
+   dspi->dma->tx_dma_buf[i] = dspi_data_to_pushr(dspi, tx_word);
+   if ((dspi->cs_change) && (!dspi->len))
+   dspi->dma->tx_dma_buf[i] &= ~SPI_PUSHR_CONT;
}
 
-   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
-   dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
-   SPI_PUSHR_PCS(dspi->cs) |
-   SPI_PUSHR_CTAS(0);
-   dspi->tx += tx_word + 1;
-
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
dma->curr_xfer_len *
@@ -351,7 +344,6 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
curr_remaining_bytes = 0;
-   dspi->len = curr_remaining_bytes;
}
}
 
-- 
2.10.2

[PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation

[Sanchayan Maity]

Hello,

This v3 set of patches have fixes for Vybrid SPI DMA
implementation and is rebased on top of latest topic/fsl-dspi. 

http://git.kernel.org/cgit/linux/kernel/git/broonie/spi.git/log/?h=topic/fsl-dspi

The patches have been tested on a Toradex Colibri Vybrid VF61 module
and now incoporate feedback from Stefan on version 2 of patchset.

Changes since v2:
1. Drop the patch "Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE"
since it's now applied and rebase the whole patchset
2. Second patch in this series "Fix continuous selection format" now
fixes the issue using an existing function and handling it similar to
existing EOQ mode and also cleaning up nicely the transmit code path.
3. Third patch now just fixes the incorrect freeing of DMA allocated buffers
and drops the minor clean up patch from earlier series completely due to
the clean up from 2 above.

Changes since v1:
1. Place the continuous selection format patch second in order and remove
code duplication
2. Improve the use of curr_xfer_len and instead of converting from bytes
to DMA transfers in every use, do it at a single place. Accordingly change
it's use at other places
3. Code cleanup patch has less to clean with change above

v2:
https://www.spinics.net/lists/arm-kernel/msg543941.html

v1:
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274632.html

Thanks & Regards,
Sanchayan.

Sanchayan Maity (3):
  spi: spi-fsl-dspi: Fix incorrect DMA setup
  spi: spi-fsl-dspi: Fix continuous selection format
  spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers

 drivers/spi/spi-fsl-dspi.c | 59 +-
 1 file changed, 27 insertions(+), 32 deletions(-)

-- 
2.10.2

[PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers

[Sanchayan Maity]

Buffers allocated with a call to dma_alloc_coherent should be
freed with dma_free_coherent instead of the currently used
devm_kfree.

Signed-off-by: Sanchayan Maity 
---
 drivers/spi/spi-fsl-dspi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 8af3151..7ada112 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -420,9 +420,11 @@ static int dspi_request_dma(struct fsl_dspi *dspi, 
phys_addr_t phy_addr)
return 0;
 
 err_slave_config:
-   devm_kfree(dev, dma->rx_dma_buf);
+   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+   dma->rx_dma_buf, dma->rx_dma_phys);
 err_rx_dma_buf:
-   devm_kfree(dev, dma->tx_dma_buf);
+   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+   dma->tx_dma_buf, dma->tx_dma_phys);
 err_tx_dma_buf:
dma_release_channel(dma->chan_tx);
 err_tx_channel:
-- 
2.10.2

[PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup

[Sanchayan Maity]

Currently dmaengine_prep_slave_single was being called with length
set to the complete DMA buffer size. This resulted in unwanted bytes
being transferred to the SPI register leading to clock and MOSI lines
having unwanted data even after chip select got deasserted and the
required bytes having been transferred.

While at it also clean up the use of curr_xfer_len which is central
to the DMA setup, from bytes to DMA transfers for every use.

Signed-off-by: Sanchayan Maity 
Reviewed-by: Stefan Agner 
---
 drivers/spi/spi-fsl-dspi.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b1ee1f5..911aadb 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -151,6 +151,7 @@ static const struct fsl_dspi_devtype_data ls2085a_data = {
 };
 
 struct fsl_dspi_dma {
+   /* Length of transfer in words of DSPI_FIFO_SIZE */
u32 curr_xfer_len;
 
u32 *tx_dma_buf;
@@ -217,15 +218,13 @@ static void dspi_rx_dma_callback(void *arg)
struct fsl_dspi *dspi = arg;
struct fsl_dspi_dma *dma = dspi->dma;
int rx_word;
-   int i, len;
+   int i;
u16 d;
 
rx_word = is_double_byte_mode(dspi);
 
-   len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
-   for (i = 0; i < len; i++) {
+   for (i = 0; i < dma->curr_xfer_len; i++) {
d = dspi->dma->rx_dma_buf[i];
rx_word ? (*(u16 *)dspi->rx = d) :
(*(u8 *)dspi->rx = d);
@@ -242,14 +241,12 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
*dspi)
struct device *dev = >pdev->dev;
int time_left;
int tx_word;
-   int i, len;
+   int i;
u16 val;
 
tx_word = is_double_byte_mode(dspi);
 
-   len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
-   for (i = 0; i < len - 1; i++) {
+   for (i = 0; i < dma->curr_xfer_len - 1; i++) {
val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
dspi->dma->tx_dma_buf[i] =
SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
@@ -265,7 +262,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
-   DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
+   dma->curr_xfer_len *
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->tx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -281,7 +280,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 
dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
dma->rx_dma_phys,
-   DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
+   dma->curr_xfer_len *
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->rx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -328,17 +329,17 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
struct device *dev = >pdev->dev;
int curr_remaining_bytes;
int bytes_per_buffer;
-   int tx_word;
+   int word = 1;
int ret = 0;
 
-   tx_word = is_double_byte_mode(dspi);
+   if (is_double_byte_mode(dspi))
+   word = 2;
curr_remaining_bytes = dspi->len;
+   bytes_per_buffer = DSPI_DMA_BUFSIZE / DSPI_FIFO_SIZE;
while (curr_remaining_bytes) {
/* Check if current transfer fits the DMA buffer */
-   dma->curr_xfer_len = curr_remaining_bytes;
-   bytes_per_buffer = DSPI_DMA_BUFSIZE /
-   (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
-   if (curr_remaining_bytes > bytes_per_buffer)
+   dma->curr_xfer_len = curr_remaining_bytes / word;
+   if (dma->curr_xfer_len > bytes_per_buffer)
dma->curr_xfer_len = bytes_per_buffer;
 
ret = dspi_next_xfer_dma_submit(dspi);
@@ -347,7 +348,7 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
goto exit;
 
} else {
-   curr_remaining_bytes -= dma->curr_xfer_len;
+   curr_remaining_bytes -= dma->curr_xfer_len * word;
if 

[PATCH v3 0/3] Fixes for Vybrid SPI DMA implementation

[Sanchayan Maity]

Hello,

This v3 set of patches have fixes for Vybrid SPI DMA
implementation and is rebased on top of latest topic/fsl-dspi. 

http://git.kernel.org/cgit/linux/kernel/git/broonie/spi.git/log/?h=topic/fsl-dspi

The patches have been tested on a Toradex Colibri Vybrid VF61 module
and now incoporate feedback from Stefan on version 2 of patchset.

Changes since v2:
1. Drop the patch "Fix SPI transfer issue when using multiple SPI_IOC_MESSAGE"
since it's now applied and rebase the whole patchset
2. Second patch in this series "Fix continuous selection format" now
fixes the issue using an existing function and handling it similar to
existing EOQ mode and also cleaning up nicely the transmit code path.
3. Third patch now just fixes the incorrect freeing of DMA allocated buffers
and drops the minor clean up patch from earlier series completely due to
the clean up from 2 above.

Changes since v1:
1. Place the continuous selection format patch second in order and remove
code duplication
2. Improve the use of curr_xfer_len and instead of converting from bytes
to DMA transfers in every use, do it at a single place. Accordingly change
it's use at other places
3. Code cleanup patch has less to clean with change above

v2:
https://www.spinics.net/lists/arm-kernel/msg543941.html

v1:
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274632.html

Thanks & Regards,
Sanchayan.

Sanchayan Maity (3):
  spi: spi-fsl-dspi: Fix incorrect DMA setup
  spi: spi-fsl-dspi: Fix continuous selection format
  spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers

 drivers/spi/spi-fsl-dspi.c | 59 +-
 1 file changed, 27 insertions(+), 32 deletions(-)

-- 
2.10.2

[PATCH v3 3/3] spi: spi-fsl-dspi: Fix incorrect freeing of DMA allocated buffers

[Sanchayan Maity]

Buffers allocated with a call to dma_alloc_coherent should be
freed with dma_free_coherent instead of the currently used
devm_kfree.

Signed-off-by: Sanchayan Maity 
---
 drivers/spi/spi-fsl-dspi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 8af3151..7ada112 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -420,9 +420,11 @@ static int dspi_request_dma(struct fsl_dspi *dspi, 
phys_addr_t phy_addr)
return 0;
 
 err_slave_config:
-   devm_kfree(dev, dma->rx_dma_buf);
+   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+   dma->rx_dma_buf, dma->rx_dma_phys);
 err_rx_dma_buf:
-   devm_kfree(dev, dma->tx_dma_buf);
+   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
+   dma->tx_dma_buf, dma->tx_dma_phys);
 err_tx_dma_buf:
dma_release_channel(dma->chan_tx);
 err_tx_channel:
-- 
2.10.2

[PATCH v3 1/3] spi: spi-fsl-dspi: Fix incorrect DMA setup

[Sanchayan Maity]

Currently dmaengine_prep_slave_single was being called with length
set to the complete DMA buffer size. This resulted in unwanted bytes
being transferred to the SPI register leading to clock and MOSI lines
having unwanted data even after chip select got deasserted and the
required bytes having been transferred.

While at it also clean up the use of curr_xfer_len which is central
to the DMA setup, from bytes to DMA transfers for every use.

Signed-off-by: Sanchayan Maity 
Reviewed-by: Stefan Agner 
---
 drivers/spi/spi-fsl-dspi.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b1ee1f5..911aadb 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -151,6 +151,7 @@ static const struct fsl_dspi_devtype_data ls2085a_data = {
 };
 
 struct fsl_dspi_dma {
+   /* Length of transfer in words of DSPI_FIFO_SIZE */
u32 curr_xfer_len;
 
u32 *tx_dma_buf;
@@ -217,15 +218,13 @@ static void dspi_rx_dma_callback(void *arg)
struct fsl_dspi *dspi = arg;
struct fsl_dspi_dma *dma = dspi->dma;
int rx_word;
-   int i, len;
+   int i;
u16 d;
 
rx_word = is_double_byte_mode(dspi);
 
-   len = rx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
-   for (i = 0; i < len; i++) {
+   for (i = 0; i < dma->curr_xfer_len; i++) {
d = dspi->dma->rx_dma_buf[i];
rx_word ? (*(u16 *)dspi->rx = d) :
(*(u8 *)dspi->rx = d);
@@ -242,14 +241,12 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
*dspi)
struct device *dev = >pdev->dev;
int time_left;
int tx_word;
-   int i, len;
+   int i;
u16 val;
 
tx_word = is_double_byte_mode(dspi);
 
-   len = tx_word ? (dma->curr_xfer_len / 2) : dma->curr_xfer_len;
-
-   for (i = 0; i < len - 1; i++) {
+   for (i = 0; i < dma->curr_xfer_len - 1; i++) {
val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
dspi->dma->tx_dma_buf[i] =
SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
@@ -265,7 +262,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
-   DSPI_DMA_BUFSIZE, DMA_MEM_TO_DEV,
+   dma->curr_xfer_len *
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->tx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -281,7 +280,9 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
 
dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
dma->rx_dma_phys,
-   DSPI_DMA_BUFSIZE, DMA_DEV_TO_MEM,
+   dma->curr_xfer_len *
+   DMA_SLAVE_BUSWIDTH_4_BYTES,
+   DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!dma->rx_desc) {
dev_err(dev, "Not able to get desc for DMA xfer\n");
@@ -328,17 +329,17 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
struct device *dev = >pdev->dev;
int curr_remaining_bytes;
int bytes_per_buffer;
-   int tx_word;
+   int word = 1;
int ret = 0;
 
-   tx_word = is_double_byte_mode(dspi);
+   if (is_double_byte_mode(dspi))
+   word = 2;
curr_remaining_bytes = dspi->len;
+   bytes_per_buffer = DSPI_DMA_BUFSIZE / DSPI_FIFO_SIZE;
while (curr_remaining_bytes) {
/* Check if current transfer fits the DMA buffer */
-   dma->curr_xfer_len = curr_remaining_bytes;
-   bytes_per_buffer = DSPI_DMA_BUFSIZE /
-   (DSPI_FIFO_SIZE / (tx_word ? 2 : 1));
-   if (curr_remaining_bytes > bytes_per_buffer)
+   dma->curr_xfer_len = curr_remaining_bytes / word;
+   if (dma->curr_xfer_len > bytes_per_buffer)
dma->curr_xfer_len = bytes_per_buffer;
 
ret = dspi_next_xfer_dma_submit(dspi);
@@ -347,7 +348,7 @@ static int dspi_dma_xfer(struct fsl_dspi *dspi)
goto exit;
 
} else {
-   curr_remaining_bytes -= dma->curr_xfer_len;
+   curr_remaining_bytes -= dma->curr_xfer_len * word;
if (curr_remaining_bytes < 0)
  

Re: [PATCHSET 0/7] perf sched: Introduce timehist command, again (v2)

[Namhyung Kim]

Hi Arnaldo,

Could you please take a look at this?

Thanks,
Namhyung


On Wed, Nov 16, 2016 at 03:06:26PM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset is a rebased version of David's sched timehist work [1].
> I plan to improve perf sched command more and think that having
> timehist command before the work looks good.  It seems David is busy
> these days, so I'm retrying it by myself.
> 
>  * changes in v2)
>   - change name 'b/n time' to 'wait time'  (Ingo)
>   - show arrow between functions in the callchain  (Ingo)
>   - fix a bug in calculating initial run time
>   
> This implements only basic feature and a few options.  I just split
> the patch to make it easier to review and did some cosmetic changes.
> More patches will come later.
> 
> The below is from the David's original description (w/ slight change):
> 
> 8<-
> 'perf sched timehist' provides an analysis of scheduling events.
> 
> Example usage:
> perf sched record -- sleep 1
> perf sched timehist
> 
> By default it shows the individual schedule events, including the time between
> sched-in events for the task, the task scheduling delay (time between wakeup
> and actually running) and run time for the task:
> 
>time cpu  task name[tid/pid]   wait time sch delay  run time
>   -   - - -
>79371.874569 [11] gcc[31949]   0.014 0.000 1.148
>79371.874591 [10] gcc[31951]   0.000 0.000 0.024
>79371.874603 [10] migration/10[59] 3.350 0.004 0.011
>79371.874604 [11]1.148 0.000 0.035
>79371.874723 [05]0.016 0.000 1.383
>79371.874746 [05] gcc[31949]   0.153 0.078 0.022
> ...
> 
> Times are in msec.usec.
> 
> If callchains were recorded they are appended to the line with a default 
> stack depth of 5:
> 
>79371.874569 [11] gcc[31949]   0.014 0.000 1.148  
> wait_for_completion_killable <- do_fork <- sys_vfork <- stub_vfork <- __vfork
>79371.874591 [10] gcc[31951]   0.000 0.000 0.024  
> __cond_resched <- _cond_resched <- wait_for_completion <- stop_one_cpu <- 
> sched_exec
>79371.874603 [10] migration/10[59] 3.350 0.004 0.011  
> smpboot_thread_fn <- kthread <- ret_from_fork
>79371.874604 [11]1.148 0.000 0.035  
> cpu_startup_entry <- start_secondary
>79371.874723 [05]0.016 0.000 1.383  
> cpu_startup_entry <- start_secondary
>79371.874746 [05] gcc[31949]   0.153 0.078 0.022  
> do_wait sys_wait4 <- system_call_fastpath <- __GI___waitpid
> 
> --no-call-graph can be used to not show the callchains. --max-stack is used
> to control the number of frames shown (default of 5). -x/--excl options can
> be used to collapse redundant callchains to get more relevant data on screen.
> 
> Similar to perf-trace -s and -S can be used to dump a statistical summary
> without or with events (respectively). Statistics include min run time,
> average run time and max run time. Stats are also shown for run time by
> cpu.
> 
> The cpu-visual option provides a visual aid for sched switches by cpu:
> ...
>79371.874569 [11]s  gcc[31949]  0.014 
> 0.000 1.148
>79371.874591 [10]   s   gcc[31951]  0.000 
> 0.000 0.024
>79371.874603 [10]   s   migration/10[59]3.350 
> 0.004 0.011
>79371.874604 [11]i1.148 
> 0.000 0.035
>79371.874723 [05]  i  0.016 
> 0.000 1.383
>79371.874746 [05]  sgcc[31949]  0.153 
> 0.078 0.022
> ...
> 
> 8<-
> 
> This code is available at 'perf/timehist-v2' branch in my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any feedback is welcomed.
> 
> Thanks,
> Namhyung
> 
> 
> [1] https://lkml.org/lkml/2013/12/1/129
> 
> David Ahern (6):
>   perf sched timehist: Introduce timehist command
>   perf sched timehist: Add summary options
>   perf sched timehist: Add -w/--wakeups option
>   perf sched timehist: Add call graph options
>   perf sched timehist: Add -V/--cpu-visual option
>   perf sched: Add documentation for timehist options
> 
> Namhyung Kim (2):
>   perf symbol: Print symbol offsets conditionally
>   perf tools: Support printing callchains with arrows
> 
>  tools/perf/Documentation/perf-sched.txt |  46 +-
>  tools/perf/builtin-sched.c  | 914 
> +++-
>  tools/perf/util/evsel.h |   1 +
>  tools/perf/util/evsel_fprintf.c |  12 +-
>  tools/perf/util/symbol.h|   3 +-
>  

Re: [PATCHSET 0/7] perf sched: Introduce timehist command, again (v2)

[Namhyung Kim]

Hi Arnaldo,

Could you please take a look at this?

Thanks,
Namhyung


On Wed, Nov 16, 2016 at 03:06:26PM +0900, Namhyung Kim wrote:
> Hello,
> 
> This patchset is a rebased version of David's sched timehist work [1].
> I plan to improve perf sched command more and think that having
> timehist command before the work looks good.  It seems David is busy
> these days, so I'm retrying it by myself.
> 
>  * changes in v2)
>   - change name 'b/n time' to 'wait time'  (Ingo)
>   - show arrow between functions in the callchain  (Ingo)
>   - fix a bug in calculating initial run time
>   
> This implements only basic feature and a few options.  I just split
> the patch to make it easier to review and did some cosmetic changes.
> More patches will come later.
> 
> The below is from the David's original description (w/ slight change):
> 
> 8<-
> 'perf sched timehist' provides an analysis of scheduling events.
> 
> Example usage:
> perf sched record -- sleep 1
> perf sched timehist
> 
> By default it shows the individual schedule events, including the time between
> sched-in events for the task, the task scheduling delay (time between wakeup
> and actually running) and run time for the task:
> 
>time cpu  task name[tid/pid]   wait time sch delay  run time
>   -   - - -
>79371.874569 [11] gcc[31949]   0.014 0.000 1.148
>79371.874591 [10] gcc[31951]   0.000 0.000 0.024
>79371.874603 [10] migration/10[59] 3.350 0.004 0.011
>79371.874604 [11]1.148 0.000 0.035
>79371.874723 [05]0.016 0.000 1.383
>79371.874746 [05] gcc[31949]   0.153 0.078 0.022
> ...
> 
> Times are in msec.usec.
> 
> If callchains were recorded they are appended to the line with a default 
> stack depth of 5:
> 
>79371.874569 [11] gcc[31949]   0.014 0.000 1.148  
> wait_for_completion_killable <- do_fork <- sys_vfork <- stub_vfork <- __vfork
>79371.874591 [10] gcc[31951]   0.000 0.000 0.024  
> __cond_resched <- _cond_resched <- wait_for_completion <- stop_one_cpu <- 
> sched_exec
>79371.874603 [10] migration/10[59] 3.350 0.004 0.011  
> smpboot_thread_fn <- kthread <- ret_from_fork
>79371.874604 [11]1.148 0.000 0.035  
> cpu_startup_entry <- start_secondary
>79371.874723 [05]0.016 0.000 1.383  
> cpu_startup_entry <- start_secondary
>79371.874746 [05] gcc[31949]   0.153 0.078 0.022  
> do_wait sys_wait4 <- system_call_fastpath <- __GI___waitpid
> 
> --no-call-graph can be used to not show the callchains. --max-stack is used
> to control the number of frames shown (default of 5). -x/--excl options can
> be used to collapse redundant callchains to get more relevant data on screen.
> 
> Similar to perf-trace -s and -S can be used to dump a statistical summary
> without or with events (respectively). Statistics include min run time,
> average run time and max run time. Stats are also shown for run time by
> cpu.
> 
> The cpu-visual option provides a visual aid for sched switches by cpu:
> ...
>79371.874569 [11]s  gcc[31949]  0.014 
> 0.000 1.148
>79371.874591 [10]   s   gcc[31951]  0.000 
> 0.000 0.024
>79371.874603 [10]   s   migration/10[59]3.350 
> 0.004 0.011
>79371.874604 [11]i1.148 
> 0.000 0.035
>79371.874723 [05]  i  0.016 
> 0.000 1.383
>79371.874746 [05]  sgcc[31949]  0.153 
> 0.078 0.022
> ...
> 
> 8<-
> 
> This code is available at 'perf/timehist-v2' branch in my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Any feedback is welcomed.
> 
> Thanks,
> Namhyung
> 
> 
> [1] https://lkml.org/lkml/2013/12/1/129
> 
> David Ahern (6):
>   perf sched timehist: Introduce timehist command
>   perf sched timehist: Add summary options
>   perf sched timehist: Add -w/--wakeups option
>   perf sched timehist: Add call graph options
>   perf sched timehist: Add -V/--cpu-visual option
>   perf sched: Add documentation for timehist options
> 
> Namhyung Kim (2):
>   perf symbol: Print symbol offsets conditionally
>   perf tools: Support printing callchains with arrows
> 
>  tools/perf/Documentation/perf-sched.txt |  46 +-
>  tools/perf/builtin-sched.c  | 914 
> +++-
>  tools/perf/util/evsel.h |   1 +
>  tools/perf/util/evsel_fprintf.c |  12 +-
>  tools/perf/util/symbol.h|   3 +-
>  

Re: [PATCHSET 0/7] perf sched: Introduce timehist command, again (v2)

[Namhyung Kim]

Hi Jiri,

On Wed, Nov 16, 2016 at 04:34:28PM +0100, Jiri Olsa wrote:
> On Wed, Nov 16, 2016 at 03:06:26PM +0900, Namhyung Kim wrote:
> > Hello,
> > 
> > This patchset is a rebased version of David's sched timehist work [1].
> > I plan to improve perf sched command more and think that having
> > timehist command before the work looks good.  It seems David is busy
> > these days, so I'm retrying it by myself.
> > 
> >  * changes in v2)
> >   - change name 'b/n time' to 'wait time'  (Ingo)
> >   - show arrow between functions in the callchain  (Ingo)
> >   - fix a bug in calculating initial run time
> 
> you might want to add some column width cuts.. I've got this:

The task name column width is initially set to 20, and grow
dynamically when longer name + tid/pid is found.  Maybe I need to
change it to have the max width initially.. (in later patch).

> 
>timecpu  task name wait time  sch delay   run time
> [tid/pid](msec) (msec) (msec)
> --- --    -  -  -
>   482381.387934 []  perf[6870]0.000  0.000  0.000 
>   482381.388838 [0002]  0.000  0.000  0.000 
>   482381.388845 [0002]  plugin-containe[25990/25970]  0.000  0.004
>   0.007 
>   482381.388897 [0003]  0.000  0.000
>   0.000 
>   482381.388919 [0002]  0.007  0.000
>   0.074 
>   482381.388925 [0003]  Timer[15210/15178]0.000  0.002
>   0.027 
> 
> 
> 
> other than that:
> 
> Acked-by: Jiri Olsa 

Thanks,
Namhyung

Re: [PATCHSET 0/7] perf sched: Introduce timehist command, again (v2)

[Namhyung Kim]

Hi Jiri,

On Wed, Nov 16, 2016 at 04:34:28PM +0100, Jiri Olsa wrote:
> On Wed, Nov 16, 2016 at 03:06:26PM +0900, Namhyung Kim wrote:
> > Hello,
> > 
> > This patchset is a rebased version of David's sched timehist work [1].
> > I plan to improve perf sched command more and think that having
> > timehist command before the work looks good.  It seems David is busy
> > these days, so I'm retrying it by myself.
> > 
> >  * changes in v2)
> >   - change name 'b/n time' to 'wait time'  (Ingo)
> >   - show arrow between functions in the callchain  (Ingo)
> >   - fix a bug in calculating initial run time
> 
> you might want to add some column width cuts.. I've got this:

The task name column width is initially set to 20, and grow
dynamically when longer name + tid/pid is found.  Maybe I need to
change it to have the max width initially.. (in later patch).

> 
>timecpu  task name wait time  sch delay   run time
> [tid/pid](msec) (msec) (msec)
> --- --    -  -  -
>   482381.387934 []  perf[6870]0.000  0.000  0.000 
>   482381.388838 [0002]  0.000  0.000  0.000 
>   482381.388845 [0002]  plugin-containe[25990/25970]  0.000  0.004
>   0.007 
>   482381.388897 [0003]  0.000  0.000
>   0.000 
>   482381.388919 [0002]  0.007  0.000
>   0.074 
>   482381.388925 [0003]  Timer[15210/15178]0.000  0.002
>   0.027 
> 
> 
> 
> other than that:
> 
> Acked-by: Jiri Olsa 

Thanks,
Namhyung

Re: [PATCH 2/2] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Namhyung Kim]

On Mon, Nov 21, 2016 at 11:41:49AM -0500, Steven Rostedt wrote:
> On Fri, 18 Nov 2016 20:40:22 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Fri, Nov 18, 2016 at 05:54:01PM -0500, Steven Rostedt escreveu:
> > > From: "Steven Rostedt (Red Hat)" 
> > > 
> > > Instead of using 100, define a USECS_PER_SEC macro and use that 
> > > instead.  
> > 
> > We already have it in tools/include/linux/time64.h :-)
> 
> Here's v2 then. Can you take both patches?
> 
> -- Steve
> 
> 
> From 856b584cb783b658d200aa3fca8faba58d52ae82 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" 
> Date: Tue, 9 Feb 2016 15:40:15 -0500
> Subject: [PATCH] tools lib traceevent: Use USEC_PER_SEC instead of hardcoded
>  number
> 
> Instead of using 100, use the define in time64.h instead.
> 
> Also remove the the duplicate defines for NSECS_PER_SEC.
> 
> Link: http://lkml.kernel.org/r/20160209204237.006667...@goodmis.org
> 
> Signed-off-by: Steven Rostedt 

Acked-by: Namhyung Kim 

Thanks,
Namhyung


> ---
>  tools/lib/traceevent/event-parse.c | 11 ++-
>  tools/lib/traceevent/event-parse.h |  3 ---
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index d6f3cc0a29b0..8206227866dc 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include "event-parse.h"
> @@ -5450,8 +5451,8 @@ void pevent_print_event_time(struct pevent *pevent, 
> struct trace_seq *s,
>   use_usec_format = is_timestamp_in_us(pevent->trace_clock,
>   use_trace_clock);
>   if (use_usec_format) {
> - secs = record->ts / NSECS_PER_SEC;
> - nsecs = record->ts - secs * NSECS_PER_SEC;
> + secs = record->ts / NSEC_PER_SEC;
> + nsecs = record->ts - secs * NSEC_PER_SEC;
>   }
>  
>   if (pevent->latency_format) {
> @@ -5463,10 +5464,10 @@ void pevent_print_event_time(struct pevent *pevent, 
> struct trace_seq *s,
>   usecs = nsecs;
>   p = 9;
>   } else {
> - usecs = (nsecs + 500) / NSECS_PER_USEC;
> + usecs = (nsecs + 500) / NSEC_PER_USEC;
>   /* To avoid usecs larger than 1 sec */
> - if (usecs >= 100) {
> - usecs -= 100;
> + if (usecs >= USEC_PER_SEC) {
> + usecs -= USEC_PER_SEC;
>   secs++;
>   }
>   p = 6;
> diff --git a/tools/lib/traceevent/event-parse.h 
> b/tools/lib/traceevent/event-parse.h
> index 41b3d2238ed3..dfa06f4e7abf 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -172,9 +172,6 @@ struct pevent_plugin_option {
>  #define PEVENT_PLUGIN_OPTIONS_NAME MAKE_STR(PEVENT_PLUGIN_OPTIONS)
>  #define PEVENT_PLUGIN_ALIAS_NAME MAKE_STR(PEVENT_PLUGIN_ALIAS)
>  
> -#define NSECS_PER_SEC10ULL
> -#define NSECS_PER_USEC   1000ULL
> -
>  enum format_flags {
>   FIELD_IS_ARRAY  = 1,
>   FIELD_IS_POINTER= 2,
> -- 
> 2.1.0
> 

Re: [PATCH 2/2] tools lib traceevent: Use USECS_PER_SEC instead of hardcoded number

[Namhyung Kim]

On Mon, Nov 21, 2016 at 11:41:49AM -0500, Steven Rostedt wrote:
> On Fri, 18 Nov 2016 20:40:22 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Fri, Nov 18, 2016 at 05:54:01PM -0500, Steven Rostedt escreveu:
> > > From: "Steven Rostedt (Red Hat)" 
> > > 
> > > Instead of using 100, define a USECS_PER_SEC macro and use that 
> > > instead.  
> > 
> > We already have it in tools/include/linux/time64.h :-)
> 
> Here's v2 then. Can you take both patches?
> 
> -- Steve
> 
> 
> From 856b584cb783b658d200aa3fca8faba58d52ae82 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" 
> Date: Tue, 9 Feb 2016 15:40:15 -0500
> Subject: [PATCH] tools lib traceevent: Use USEC_PER_SEC instead of hardcoded
>  number
> 
> Instead of using 100, use the define in time64.h instead.
> 
> Also remove the the duplicate defines for NSECS_PER_SEC.
> 
> Link: http://lkml.kernel.org/r/20160209204237.006667...@goodmis.org
> 
> Signed-off-by: Steven Rostedt 

Acked-by: Namhyung Kim 

Thanks,
Namhyung


> ---
>  tools/lib/traceevent/event-parse.c | 11 ++-
>  tools/lib/traceevent/event-parse.h |  3 ---
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index d6f3cc0a29b0..8206227866dc 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include "event-parse.h"
> @@ -5450,8 +5451,8 @@ void pevent_print_event_time(struct pevent *pevent, 
> struct trace_seq *s,
>   use_usec_format = is_timestamp_in_us(pevent->trace_clock,
>   use_trace_clock);
>   if (use_usec_format) {
> - secs = record->ts / NSECS_PER_SEC;
> - nsecs = record->ts - secs * NSECS_PER_SEC;
> + secs = record->ts / NSEC_PER_SEC;
> + nsecs = record->ts - secs * NSEC_PER_SEC;
>   }
>  
>   if (pevent->latency_format) {
> @@ -5463,10 +5464,10 @@ void pevent_print_event_time(struct pevent *pevent, 
> struct trace_seq *s,
>   usecs = nsecs;
>   p = 9;
>   } else {
> - usecs = (nsecs + 500) / NSECS_PER_USEC;
> + usecs = (nsecs + 500) / NSEC_PER_USEC;
>   /* To avoid usecs larger than 1 sec */
> - if (usecs >= 100) {
> - usecs -= 100;
> + if (usecs >= USEC_PER_SEC) {
> + usecs -= USEC_PER_SEC;
>   secs++;
>   }
>   p = 6;
> diff --git a/tools/lib/traceevent/event-parse.h 
> b/tools/lib/traceevent/event-parse.h
> index 41b3d2238ed3..dfa06f4e7abf 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -172,9 +172,6 @@ struct pevent_plugin_option {
>  #define PEVENT_PLUGIN_OPTIONS_NAME MAKE_STR(PEVENT_PLUGIN_OPTIONS)
>  #define PEVENT_PLUGIN_ALIAS_NAME MAKE_STR(PEVENT_PLUGIN_ALIAS)
>  
> -#define NSECS_PER_SEC10ULL
> -#define NSECS_PER_USEC   1000ULL
> -
>  enum format_flags {
>   FIELD_IS_ARRAY  = 1,
>   FIELD_IS_POINTER= 2,
> -- 
> 2.1.0
> 

Re: [PATCH 1/2] tools lib traceevent: Add retrieval of preempt count and latency flags

[Namhyung Kim]

Hi Steve,

On Fri, Nov 18, 2016 at 05:54:00PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" 
> 
> Add a way to retrieve the preempt count as well as the latency flags from a
> pevent_record.
> 
>  int pevent_data_pc(pevent, record);

I think pevent_data_preempt_count() is better..

Thanks,
Namhyung


> 
> returns the preempt count of a record.
> 
>  int pevent_data_flags(pevent, record);
> 
> returns the latency flags for a record.
> 
> Signed-off-by: Steven Rostedt 
> ---
>  tools/lib/traceevent/event-parse.c | 30 --
>  tools/lib/traceevent/event-parse.h |  2 ++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index 664c90c8e22b..d6f3cc0a29b0 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -5191,11 +5191,11 @@ struct event_format 
> *pevent_data_event_from_type(struct pevent *pevent, int type
>  }
>  
>  /**
> - * pevent_data_pid - parse the PID from raw data
> + * pevent_data_pid - parse the PID from record
>   * @pevent: a handle to the pevent
>   * @rec: the record to parse
>   *
> - * This returns the PID from a raw data.
> + * This returns the PID from a record.
>   */
>  int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec)
>  {
> @@ -5203,6 +5203,32 @@ int pevent_data_pid(struct pevent *pevent, struct 
> pevent_record *rec)
>  }
>  
>  /**
> + * pevent_data_pc - parse the preempt count from the record
> + * @pevent: a handle to the pevent
> + * @rec: the record to parse
> + *
> + * This returns the preempt count from a record.
> + */
> +int pevent_data_pc(struct pevent *pevent, struct pevent_record *rec)
> +{
> + return parse_common_pc(pevent, rec->data);
> +}
> +
> +/**
> + * pevent_data_flags - parse the latency flags from the record
> + * @pevent: a handle to the pevent
> + * @rec: the record to parse
> + *
> + * This returns the latency flags from a record.
> + *
> + *  Use trace_flag_type enum for the flags (see event-parse.h).
> + */
> +int pevent_data_flags(struct pevent *pevent, struct pevent_record *rec)
> +{
> + return parse_common_flags(pevent, rec->data);
> +}
> +
> +/**
>   * pevent_data_comm_from_pid - return the command line from PID
>   * @pevent: a handle to the pevent
>   * @pid: the PID of the task to search for
> diff --git a/tools/lib/traceevent/event-parse.h 
> b/tools/lib/traceevent/event-parse.h
> index 9ffde377e89d..41b3d2238ed3 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -712,6 +712,8 @@ void pevent_data_lat_fmt(struct pevent *pevent,
>  int pevent_data_type(struct pevent *pevent, struct pevent_record *rec);
>  struct event_format *pevent_data_event_from_type(struct pevent *pevent, int 
> type);
>  int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
> +int pevent_data_pc(struct pevent *pevent, struct pevent_record *rec);
> +int pevent_data_flags(struct pevent *pevent, struct pevent_record *rec);
>  const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
>  struct cmdline;
>  struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char 
> *comm,
> -- 
> 2.10.2
> 
> 

Re: [PATCH 1/2] tools lib traceevent: Add retrieval of preempt count and latency flags

[Namhyung Kim]

Hi Steve,

On Fri, Nov 18, 2016 at 05:54:00PM -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" 
> 
> Add a way to retrieve the preempt count as well as the latency flags from a
> pevent_record.
> 
>  int pevent_data_pc(pevent, record);

I think pevent_data_preempt_count() is better..

Thanks,
Namhyung


> 
> returns the preempt count of a record.
> 
>  int pevent_data_flags(pevent, record);
> 
> returns the latency flags for a record.
> 
> Signed-off-by: Steven Rostedt 
> ---
>  tools/lib/traceevent/event-parse.c | 30 --
>  tools/lib/traceevent/event-parse.h |  2 ++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c 
> b/tools/lib/traceevent/event-parse.c
> index 664c90c8e22b..d6f3cc0a29b0 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -5191,11 +5191,11 @@ struct event_format 
> *pevent_data_event_from_type(struct pevent *pevent, int type
>  }
>  
>  /**
> - * pevent_data_pid - parse the PID from raw data
> + * pevent_data_pid - parse the PID from record
>   * @pevent: a handle to the pevent
>   * @rec: the record to parse
>   *
> - * This returns the PID from a raw data.
> + * This returns the PID from a record.
>   */
>  int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec)
>  {
> @@ -5203,6 +5203,32 @@ int pevent_data_pid(struct pevent *pevent, struct 
> pevent_record *rec)
>  }
>  
>  /**
> + * pevent_data_pc - parse the preempt count from the record
> + * @pevent: a handle to the pevent
> + * @rec: the record to parse
> + *
> + * This returns the preempt count from a record.
> + */
> +int pevent_data_pc(struct pevent *pevent, struct pevent_record *rec)
> +{
> + return parse_common_pc(pevent, rec->data);
> +}
> +
> +/**
> + * pevent_data_flags - parse the latency flags from the record
> + * @pevent: a handle to the pevent
> + * @rec: the record to parse
> + *
> + * This returns the latency flags from a record.
> + *
> + *  Use trace_flag_type enum for the flags (see event-parse.h).
> + */
> +int pevent_data_flags(struct pevent *pevent, struct pevent_record *rec)
> +{
> + return parse_common_flags(pevent, rec->data);
> +}
> +
> +/**
>   * pevent_data_comm_from_pid - return the command line from PID
>   * @pevent: a handle to the pevent
>   * @pid: the PID of the task to search for
> diff --git a/tools/lib/traceevent/event-parse.h 
> b/tools/lib/traceevent/event-parse.h
> index 9ffde377e89d..41b3d2238ed3 100644
> --- a/tools/lib/traceevent/event-parse.h
> +++ b/tools/lib/traceevent/event-parse.h
> @@ -712,6 +712,8 @@ void pevent_data_lat_fmt(struct pevent *pevent,
>  int pevent_data_type(struct pevent *pevent, struct pevent_record *rec);
>  struct event_format *pevent_data_event_from_type(struct pevent *pevent, int 
> type);
>  int pevent_data_pid(struct pevent *pevent, struct pevent_record *rec);
> +int pevent_data_pc(struct pevent *pevent, struct pevent_record *rec);
> +int pevent_data_flags(struct pevent *pevent, struct pevent_record *rec);
>  const char *pevent_data_comm_from_pid(struct pevent *pevent, int pid);
>  struct cmdline;
>  struct cmdline *pevent_data_pid_from_comm(struct pevent *pevent, const char 
> *comm,
> -- 
> 2.10.2
> 
> 

Re: [PATCH 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI

[Adrian Hunter]

On 22/11/16 01:04, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan 
> Reviewed-by: Jaeden Amero 
> Reviewed-by: Josh Cartwright 
> Signed-off-by: Zach Brown 
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..a855c97 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -377,6 +378,20 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>  
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +#ifdef CONFIG_ACPI
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(>chip->pdev->dev),
> +"MXFQ", NULL, _freq);
> + if (ACPI_FAILURE(status)) {
> + dev_info(>chip->pdev->dev,

Doesn't it error out the whole probe.  So it should be dev_err()

> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 100;
> +#endif
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>MMC_CAP_WAIT_WHILE_BUSY;
>   return 0;
> 

Re: [PATCH 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI

[Adrian Hunter]

On 22/11/16 01:04, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
> 
> Signed-off-by: Nathan Sullivan 
> Reviewed-by: Jaeden Amero 
> Reviewed-by: Josh Cartwright 
> Signed-off-by: Zach Brown 
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c 
> b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..a855c97 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -377,6 +378,20 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot 
> *slot)
>  
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +#ifdef CONFIG_ACPI
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(>chip->pdev->dev),
> +"MXFQ", NULL, _freq);
> + if (ACPI_FAILURE(status)) {
> + dev_info(>chip->pdev->dev,

Doesn't it error out the whole probe.  So it should be dev_err()

> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 100;
> +#endif
>   slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
>MMC_CAP_WAIT_WHILE_BUSY;
>   return 0;
> 

Re: [RFC][PATCH 3/3] usb: dwc2: Make sure we disconnect the gadget state

[John Stultz]

On Mon, Nov 21, 2016 at 7:23 PM, John Youn  wrote:
> On 11/15/2016 1:47 PM, John Stultz wrote:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>
> The fifo_map could end up not being clear when disconnect is never
> sent to the UDC framework. That unsets the configuration and the
> endpoints get disabled, which clears the FIFO map.
>
> Looks like the problem happens when going from A-device to B-device.
>
> If you come up as an A-Device, the gadget wouldn't have been
> configured so it shouldn't warn going A->B.
>
> If you go B->A, you will get a session end detected, which triggers
> the udc disconnect. Then A->B should not warn here either.

Yea. From the udc perspective, it seems like we see:

usb_ep_enable()
gadget works fine...

unplug gadget cable, it switches into host mode.

re-plug gadget cable, and as it switches into gadget mode:

[   74.241737] JDB: udc-core usb_ep_disable!
[   74.245812] JDB: udc-core usb_ep_disable!
[   74.302390] dwc2 f72c.usb: new device is high-speed
[   74.474131] dwc2 f72c.usb: new device is high-speed
[   74.550185] dwc2 f72c.usb: new device is high-speed
[   74.621953] dwc2 f72c.usb: new address 74
[   74.651824] configfs-gadget gadget: high-speed config #1: b
[   74.657467] JDB: udc-core usb_ep_enable!
[   74.661422] JDB: udc-core usb_ep_enable!

This is why I suspect that the overly simplistic phy driver is part of
the problem here.  However, in trying to extend it to be more
functional (I've got extcon wired up and see events when I plug in and
out), I'm still having trouble, as its not clear how the generic phy
(not usb phy) driver is supposed to interact with the UDC/dwc2 driver.

Part of the issue I've seen is that while the hikey generic phy driver
tries to mimic some of the usb-phy drivers, creating an otg device and
registering it:

priv->dev = >dev;
priv->phy.dev = priv->dev;
priv->phy.label = "hi6220_usb_phy";
priv->phy.otg = otg;
priv->phy.type = USB_PHY_TYPE_USB2;
otg->set_host = hi6220_otg_set_host;
otg->set_peripheral = hi6220_otg_set_peripheral;
otg->usb_phy = >phy;

platform_set_drvdata(pdev, priv);

phy_set_drvdata(phy, priv);

usb_add_phy_dev(>phy);

The trouble is the dwc2 driver doesn't seem to call
set_host/peripheral() because it never sets the uphy pointer in
dwc2_lowlevel_hw_init(), as the hikey generic phy driver is found
before that point.

So I'm not really sure how to get the generic phy -> usb_gadget
connection established so I can call
usb_gadget_vbus_connect/disconnect at the right time..

Suggestions?

> Can you determine why this doesn't happen on your system? It sounds
> like there might be some race condition that we need to identify. If
> you can provide logs with DEBUG enabled that would be helpful too.

I'll try to capture some more data. I've got my own debug printouts
littering the logs, so I'll try to pull those out of the way.

thanks
-john

Re: [RFC][PATCH 3/3] usb: dwc2: Make sure we disconnect the gadget state

[John Stultz]

On Mon, Nov 21, 2016 at 7:23 PM, John Youn  wrote:
> On 11/15/2016 1:47 PM, John Stultz wrote:
>> I had seen some odd behavior with HiKey's usb-gadget interface
>> that I finally seemed to have chased down. Basically every other
>> time I pluged in the OTG port, the gadget interface would
>> properly initialize. The other times, I'd get a big WARN_ON
>> in dwc2_hsotg_init_fifo() about the fifo_map not being clear.
>
> The fifo_map could end up not being clear when disconnect is never
> sent to the UDC framework. That unsets the configuration and the
> endpoints get disabled, which clears the FIFO map.
>
> Looks like the problem happens when going from A-device to B-device.
>
> If you come up as an A-Device, the gadget wouldn't have been
> configured so it shouldn't warn going A->B.
>
> If you go B->A, you will get a session end detected, which triggers
> the udc disconnect. Then A->B should not warn here either.

Yea. From the udc perspective, it seems like we see:

usb_ep_enable()
gadget works fine...

unplug gadget cable, it switches into host mode.

re-plug gadget cable, and as it switches into gadget mode:

[   74.241737] JDB: udc-core usb_ep_disable!
[   74.245812] JDB: udc-core usb_ep_disable!
[   74.302390] dwc2 f72c.usb: new device is high-speed
[   74.474131] dwc2 f72c.usb: new device is high-speed
[   74.550185] dwc2 f72c.usb: new device is high-speed
[   74.621953] dwc2 f72c.usb: new address 74
[   74.651824] configfs-gadget gadget: high-speed config #1: b
[   74.657467] JDB: udc-core usb_ep_enable!
[   74.661422] JDB: udc-core usb_ep_enable!

This is why I suspect that the overly simplistic phy driver is part of
the problem here.  However, in trying to extend it to be more
functional (I've got extcon wired up and see events when I plug in and
out), I'm still having trouble, as its not clear how the generic phy
(not usb phy) driver is supposed to interact with the UDC/dwc2 driver.

Part of the issue I've seen is that while the hikey generic phy driver
tries to mimic some of the usb-phy drivers, creating an otg device and
registering it:

priv->dev = >dev;
priv->phy.dev = priv->dev;
priv->phy.label = "hi6220_usb_phy";
priv->phy.otg = otg;
priv->phy.type = USB_PHY_TYPE_USB2;
otg->set_host = hi6220_otg_set_host;
otg->set_peripheral = hi6220_otg_set_peripheral;
otg->usb_phy = >phy;

platform_set_drvdata(pdev, priv);

phy_set_drvdata(phy, priv);

usb_add_phy_dev(>phy);

The trouble is the dwc2 driver doesn't seem to call
set_host/peripheral() because it never sets the uphy pointer in
dwc2_lowlevel_hw_init(), as the hikey generic phy driver is found
before that point.

So I'm not really sure how to get the generic phy -> usb_gadget
connection established so I can call
usb_gadget_vbus_connect/disconnect at the right time..

Suggestions?

> Can you determine why this doesn't happen on your system? It sounds
> like there might be some race condition that we need to identify. If
> you can provide logs with DEBUG enabled that would be helpful too.

I'll try to capture some more data. I've got my own debug printouts
littering the logs, so I'll try to pull those out of the way.

thanks
-john

[PATCH] Input: elan_i2c - Add new ic and modify some functions for new IC infomation Signed-off-by: KT Liao <kt.l...@emc.com.tw>

[KT Liao]

---
 drivers/input/mouse/elan_i2c.h   |  6 ++-
 drivers/input/mouse/elan_i2c_core.c  | 40 +--
 drivers/input/mouse/elan_i2c_i2c.c   | 74 
 drivers/input/mouse/elan_i2c_smbus.c |  9 -
 4 files changed, 108 insertions(+), 21 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c.h b/drivers/input/mouse/elan_i2c.h
index c0ec261..a1228dd 100644
--- a/drivers/input/mouse/elan_i2c.h
+++ b/drivers/input/mouse/elan_i2c.h
@@ -56,9 +56,10 @@ struct elan_transport_ops {
int (*get_baseline_data)(struct i2c_client *client,
 bool max_baseliune, u8 *value);
 
-   int (*get_version)(struct i2c_client *client, bool iap, u8 *version);
+   int (*get_version)(struct i2c_client *client,
+  bool iap, u8 *version);
int (*get_sm_version)(struct i2c_client *client,
- u8* ic_type, u8 *version);
+ u16 *ic_type, u8 *version);
int (*get_checksum)(struct i2c_client *client, bool iap, u16 *csum);
int (*get_product_id)(struct i2c_client *client, u16 *id);
 
@@ -82,6 +83,7 @@ struct elan_transport_ops {
int (*get_report)(struct i2c_client *client, u8 *report);
int (*get_pressure_adjustment)(struct i2c_client *client,
   int *adjustment);
+   int (*get_pattern)(struct i2c_client *client, u8 *pattern);
 };
 
 extern const struct elan_transport_ops elan_smbus_ops, elan_i2c_ops;
diff --git a/drivers/input/mouse/elan_i2c_core.c 
b/drivers/input/mouse/elan_i2c_core.c
index d15b338..818c73a 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -5,7 +5,7 @@
  *
  * Author: ? (Duson Lin) 
  * Author: KT Liao 
- * Version: 1.6.2
+ * Version: 1.6.3
  *
  * Based on cyapa driver:
  * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
@@ -41,7 +41,7 @@
 #include "elan_i2c.h"
 
 #define DRIVER_NAME"elan_i2c"
-#define ELAN_DRIVER_VERSION"1.6.2"
+#define ELAN_DRIVER_VERSION"1.6.3"
 #define ELAN_VENDOR_ID 0x04f3
 #define ETP_MAX_PRESSURE   255
 #define ETP_FWIDTH_REDUCE  90
@@ -78,6 +78,7 @@ struct elan_tp_data {
unsigned intx_res;
unsigned inty_res;
 
+   u8  pattern;
u16 product_id;
u8  fw_version;
u8  sm_version;
@@ -85,7 +86,7 @@ struct elan_tp_data {
u16 fw_checksum;
int pressure_adjustment;
u8  mode;
-   u8  ic_type;
+   u16 ic_type;
u16 fw_validpage_count;
u16 fw_signature_address;
 
@@ -96,10 +97,10 @@ struct elan_tp_data {
boolbaseline_ready;
 };
 
-static int elan_get_fwinfo(u8 iap_version, u16 *validpage_count,
+static int elan_get_fwinfo(u16 ic_type, u16 *validpage_count,
   u16 *signature_address)
 {
-   switch (iap_version) {
+   switch (ic_type) {
case 0x00:
case 0x06:
case 0x08:
@@ -119,6 +120,9 @@ static int elan_get_fwinfo(u8 iap_version, u16 
*validpage_count,
case 0x0E:
*validpage_count = 640;
break;
+   case 0x10:
+   *validpage_count = 1024;
+   break;
default:
/* unknown ic type clear value */
*validpage_count = 0;
@@ -209,7 +213,8 @@ static int elan_query_product(struct elan_tp_data *data)
return error;
 
error = data->ops->get_sm_version(data->client, >ic_type,
- >sm_version);
+   >sm_version);
+
if (error)
return error;
 
@@ -302,7 +307,7 @@ static int elan_initialize(struct elan_tp_data *data)
 
 static int elan_query_device_info(struct elan_tp_data *data)
 {
-   int error;
+   int error, ic_type;
 
error = data->ops->get_version(data->client, false, >fw_version);
if (error)
@@ -317,12 +322,22 @@ static int elan_query_device_info(struct elan_tp_data 
*data)
if (error)
return error;
 
+
error = data->ops->get_pressure_adjustment(data->client,
   >pressure_adjustment);
if (error)
return error;
 
-   error = elan_get_fwinfo(data->iap_version, >fw_validpage_count,
+   error = data->ops->get_pattern(data->client, >pattern);
+   if (error)
+   return error;
+
+   if (data->pattern == 0x01)
+   ic_type = data->ic_type;
+   else
+   ic_type = data->iap_version;
+
+   error = elan_get_fwinfo(ic_type, >fw_validpage_count,
 

[PATCH] Input: elan_i2c - Add new ic and modify some functions for new IC infomation Signed-off-by: KT Liao

[KT Liao]

---
 drivers/input/mouse/elan_i2c.h   |  6 ++-
 drivers/input/mouse/elan_i2c_core.c  | 40 +--
 drivers/input/mouse/elan_i2c_i2c.c   | 74 
 drivers/input/mouse/elan_i2c_smbus.c |  9 -
 4 files changed, 108 insertions(+), 21 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c.h b/drivers/input/mouse/elan_i2c.h
index c0ec261..a1228dd 100644
--- a/drivers/input/mouse/elan_i2c.h
+++ b/drivers/input/mouse/elan_i2c.h
@@ -56,9 +56,10 @@ struct elan_transport_ops {
int (*get_baseline_data)(struct i2c_client *client,
 bool max_baseliune, u8 *value);
 
-   int (*get_version)(struct i2c_client *client, bool iap, u8 *version);
+   int (*get_version)(struct i2c_client *client,
+  bool iap, u8 *version);
int (*get_sm_version)(struct i2c_client *client,
- u8* ic_type, u8 *version);
+ u16 *ic_type, u8 *version);
int (*get_checksum)(struct i2c_client *client, bool iap, u16 *csum);
int (*get_product_id)(struct i2c_client *client, u16 *id);
 
@@ -82,6 +83,7 @@ struct elan_transport_ops {
int (*get_report)(struct i2c_client *client, u8 *report);
int (*get_pressure_adjustment)(struct i2c_client *client,
   int *adjustment);
+   int (*get_pattern)(struct i2c_client *client, u8 *pattern);
 };
 
 extern const struct elan_transport_ops elan_smbus_ops, elan_i2c_ops;
diff --git a/drivers/input/mouse/elan_i2c_core.c 
b/drivers/input/mouse/elan_i2c_core.c
index d15b338..818c73a 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -5,7 +5,7 @@
  *
  * Author: ? (Duson Lin) 
  * Author: KT Liao 
- * Version: 1.6.2
+ * Version: 1.6.3
  *
  * Based on cyapa driver:
  * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
@@ -41,7 +41,7 @@
 #include "elan_i2c.h"
 
 #define DRIVER_NAME"elan_i2c"
-#define ELAN_DRIVER_VERSION"1.6.2"
+#define ELAN_DRIVER_VERSION"1.6.3"
 #define ELAN_VENDOR_ID 0x04f3
 #define ETP_MAX_PRESSURE   255
 #define ETP_FWIDTH_REDUCE  90
@@ -78,6 +78,7 @@ struct elan_tp_data {
unsigned intx_res;
unsigned inty_res;
 
+   u8  pattern;
u16 product_id;
u8  fw_version;
u8  sm_version;
@@ -85,7 +86,7 @@ struct elan_tp_data {
u16 fw_checksum;
int pressure_adjustment;
u8  mode;
-   u8  ic_type;
+   u16 ic_type;
u16 fw_validpage_count;
u16 fw_signature_address;
 
@@ -96,10 +97,10 @@ struct elan_tp_data {
boolbaseline_ready;
 };
 
-static int elan_get_fwinfo(u8 iap_version, u16 *validpage_count,
+static int elan_get_fwinfo(u16 ic_type, u16 *validpage_count,
   u16 *signature_address)
 {
-   switch (iap_version) {
+   switch (ic_type) {
case 0x00:
case 0x06:
case 0x08:
@@ -119,6 +120,9 @@ static int elan_get_fwinfo(u8 iap_version, u16 
*validpage_count,
case 0x0E:
*validpage_count = 640;
break;
+   case 0x10:
+   *validpage_count = 1024;
+   break;
default:
/* unknown ic type clear value */
*validpage_count = 0;
@@ -209,7 +213,8 @@ static int elan_query_product(struct elan_tp_data *data)
return error;
 
error = data->ops->get_sm_version(data->client, >ic_type,
- >sm_version);
+   >sm_version);
+
if (error)
return error;
 
@@ -302,7 +307,7 @@ static int elan_initialize(struct elan_tp_data *data)
 
 static int elan_query_device_info(struct elan_tp_data *data)
 {
-   int error;
+   int error, ic_type;
 
error = data->ops->get_version(data->client, false, >fw_version);
if (error)
@@ -317,12 +322,22 @@ static int elan_query_device_info(struct elan_tp_data 
*data)
if (error)
return error;
 
+
error = data->ops->get_pressure_adjustment(data->client,
   >pressure_adjustment);
if (error)
return error;
 
-   error = elan_get_fwinfo(data->iap_version, >fw_validpage_count,
+   error = data->ops->get_pattern(data->client, >pattern);
+   if (error)
+   return error;
+
+   if (data->pattern == 0x01)
+   ic_type = data->ic_type;
+   else
+   ic_type = data->iap_version;
+
+   error = elan_get_fwinfo(ic_type, >fw_validpage_count,

Re: [PATCH] zram: Fix unbalanced idr management at hot removal

[Sergey Senozhatsky]

A correction,

On (11/22/16 10:09), Sergey Senozhatsky wrote:
[..]
> > > Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=1010970
> > > Reported-and-tested-by: David Disseldorp 
> > > Reviewed-by: David Disseldorp 
> > > Cc: 
> > > Signed-off-by: Takashi Iwai 
> 
> 
> > Acked-by: Minchan Kim 
> 
> Link: lkml.kernel.org/r/20161121132140.12683-1-ti...@suse.de
> Fixes: 17ec4cd98578 ("zram: don't call idr_remove() from zram_remove()")
> Cc: [4.5+]
    4.4+


> Acked-by: Sergey Senozhatsky 



but somehow I didn't notice that Takashi Cc'd stable in the original patch.
so mine can be dropped.

-ss

Re: [PATCH] zram: Fix unbalanced idr management at hot removal

[Sergey Senozhatsky]

A correction,

On (11/22/16 10:09), Sergey Senozhatsky wrote:
[..]
> > > Bugzilla: https://bugzilla.opensuse.org/show_bug.cgi?id=1010970
> > > Reported-and-tested-by: David Disseldorp 
> > > Reviewed-by: David Disseldorp 
> > > Cc: 
> > > Signed-off-by: Takashi Iwai 
> 
> 
> > Acked-by: Minchan Kim 
> 
> Link: lkml.kernel.org/r/20161121132140.12683-1-ti...@suse.de
> Fixes: 17ec4cd98578 ("zram: don't call idr_remove() from zram_remove()")
> Cc: [4.5+]
    4.4+


> Acked-by: Sergey Senozhatsky 



but somehow I didn't notice that Takashi Cc'd stable in the original patch.
so mine can be dropped.

-ss

[PATCH] ipv6:ipv6_pinfo dereferenced after NULL check

[Manjeet Pawar]

From: Rohit Thapliyal 

np checked for NULL and then dereferenced. It should be modified
for NULL case.

Signed-off-by: Rohit Thapliyal 
Signed-off-by: Manjeet Pawar 
---
 net/ipv6/ip6_output.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 1dfc402..c2afa14 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -205,14 +205,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, 
struct flowi6 *fl6,
/*
 *  Fill in the IPv6 header
 */
-   if (np)
+   if (np) {
hlimit = np->hop_limit;
+   ip6_flow_hdr(
+   hdr, tclass, ip6_make_flowlabel(
+   net, skb, fl6->flowlabel,
+   np->autoflowlabel, fl6));
+   }
if (hlimit < 0)
hlimit = ip6_dst_hoplimit(dst);
 
-   ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
-   np->autoflowlabel, fl6));
-
hdr->payload_len = htons(seg_len);
hdr->nexthdr = proto;
hdr->hop_limit = hlimit;
-- 
1.9.1

[PATCH] ipv6:ipv6_pinfo dereferenced after NULL check

[Manjeet Pawar]

From: Rohit Thapliyal 

np checked for NULL and then dereferenced. It should be modified
for NULL case.

Signed-off-by: Rohit Thapliyal 
Signed-off-by: Manjeet Pawar 
---
 net/ipv6/ip6_output.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 1dfc402..c2afa14 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -205,14 +205,15 @@ int ip6_xmit(const struct sock *sk, struct sk_buff *skb, 
struct flowi6 *fl6,
/*
 *  Fill in the IPv6 header
 */
-   if (np)
+   if (np) {
hlimit = np->hop_limit;
+   ip6_flow_hdr(
+   hdr, tclass, ip6_make_flowlabel(
+   net, skb, fl6->flowlabel,
+   np->autoflowlabel, fl6));
+   }
if (hlimit < 0)
hlimit = ip6_dst_hoplimit(dst);
 
-   ip6_flow_hdr(hdr, tclass, ip6_make_flowlabel(net, skb, fl6->flowlabel,
-   np->autoflowlabel, fl6));
-
hdr->payload_len = htons(seg_len);
hdr->nexthdr = proto;
hdr->hop_limit = hlimit;
-- 
1.9.1

Re: [mm PATCH v3 21/23] mm: Add support for releasing multiple instances of a page

[Andrew Morton]

On Mon, 21 Nov 2016 08:21:39 -0800 Alexander Duyck  
wrote:

> >> + __free_pages_ok(page, order);
> >> + }
> >> +}
> >> +EXPORT_SYMBOL(__page_frag_drain);
> >
> > It's an exported-to-modules library function.  It should be documented,
> > please?  The page-frag API is only partially documented, but that's no
> > excuse.
> 
> Okay.  I assume you want the documentation as a follow-up patch since
> I received a notice that the patch was added to -mm?

Yes please.  Or a replacement patch which I'll temporarily turn into a
delta, either is fine.

> If you would like I could look at doing a couple of renaming patches
> so that we make the API a bit more consistent.  I could move the
> __alloc and __free to what you have suggested, and then take a look at
> trying to rename the refill/drain to be a bit more consistent in terms
> of what they are supposed to work on and how they are supposed to be
> used.

I think that would be better - it's hardly high-priority but a bit of
attention to the documentation and naming conventions would help tidy
things up.  When you can't find anything else to do ;)

Re: [mm PATCH v3 21/23] mm: Add support for releasing multiple instances of a page

[Andrew Morton]

On Mon, 21 Nov 2016 08:21:39 -0800 Alexander Duyck  
wrote:

> >> + __free_pages_ok(page, order);
> >> + }
> >> +}
> >> +EXPORT_SYMBOL(__page_frag_drain);
> >
> > It's an exported-to-modules library function.  It should be documented,
> > please?  The page-frag API is only partially documented, but that's no
> > excuse.
> 
> Okay.  I assume you want the documentation as a follow-up patch since
> I received a notice that the patch was added to -mm?

Yes please.  Or a replacement patch which I'll temporarily turn into a
delta, either is fine.

> If you would like I could look at doing a couple of renaming patches
> so that we make the API a bit more consistent.  I could move the
> __alloc and __free to what you have suggested, and then take a look at
> trying to rename the refill/drain to be a bit more consistent in terms
> of what they are supposed to work on and how they are supposed to be
> used.

I think that would be better - it's hardly high-priority but a bit of
attention to the documentation and naming conventions would help tidy
things up.  When you can't find anything else to do ;)

[GIT PULL] SCSI fixes for 4.9-rc6

[James Bottomley]

Two small fixes.  One prevents timeouts on mpt3sas when trying to use
the secure erase protocol which causes the erase protocol to be
aborted.  The second is a regression in a prior fix which causes all
commands to abort during PCI extended error recovery, which is
incorrect because PCI EEH is independent from what's happening on the
FC transport.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Andrey Grodzovsky (1):
  scsi: mpt3sas: Fix secure erase premature termination

Mauricio Faria de Oliveira (1):
  scsi: qla2xxx: do not abort all commands in the adapter during EEH 
recovery

And the diffstat:

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ++-
 drivers/scsi/qla2xxx/qla_os.c| 21 +
 2 files changed, 27 insertions(+), 9 deletions(-)

With full diff below.

James

---

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 8aa769a..91b70bc 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4010,7 +4010,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
ioc_status)
SAM_STAT_CHECK_CONDITION;
 }
 
-
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
 
 /**
  * scsih_qcmd - main scsi request entry point
@@ -4038,6 +4041,13 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
*scmd)
if (ioc->logging_level & MPT_DEBUG_SCSI)
scsi_print_command(scmd);
 
+   /*
+* Lock the device for any subsequent command until command is
+* done.
+*/
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_block(scmd->device);
+
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
scmd->result = DID_NO_CONNECT << 16;
@@ -4613,6 +4623,9 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
if (scmd == NULL)
return 1;
 
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
 
if (mpi_reply == NULL) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 567fa08..56d6142 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1456,15 +1456,20 @@ qla2x00_abort_all_cmds(scsi_qla_host_t *vha, int res)
for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
sp = req->outstanding_cmds[cnt];
if (sp) {
-   /* Get a reference to the sp and drop the lock.
-* The reference ensures this sp->done() call
-* - and not the call in qla2xxx_eh_abort() -
-* ends the SCSI command (with result 'res').
+   /* Don't abort commands in adapter during EEH
+* recovery as it's not accessible/responding.
 */
-   sp_get(sp);
-   spin_unlock_irqrestore(>hardware_lock, 
flags);
-   qla2xxx_eh_abort(GET_CMD_SP(sp));
-   spin_lock_irqsave(>hardware_lock, flags);
+   if (!ha->flags.eeh_busy) {
+   /* Get a reference to the sp and drop 
the lock.
+* The reference ensures this 
sp->done() call
+* - and not the call in 
qla2xxx_eh_abort() -
+* ends the SCSI command (with result 
'res').
+*/
+   sp_get(sp);
+   
spin_unlock_irqrestore(>hardware_lock, flags);
+   qla2xxx_eh_abort(GET_CMD_SP(sp));
+   spin_lock_irqsave(>hardware_lock, 
flags);
+   }
req->outstanding_cmds[cnt] = NULL;
sp->done(vha, sp, res);
}

[GIT PULL] SCSI fixes for 4.9-rc6

[James Bottomley]

Two small fixes.  One prevents timeouts on mpt3sas when trying to use
the secure erase protocol which causes the erase protocol to be
aborted.  The second is a regression in a prior fix which causes all
commands to abort during PCI extended error recovery, which is
incorrect because PCI EEH is independent from what's happening on the
FC transport.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Andrey Grodzovsky (1):
  scsi: mpt3sas: Fix secure erase premature termination

Mauricio Faria de Oliveira (1):
  scsi: qla2xxx: do not abort all commands in the adapter during EEH 
recovery

And the diffstat:

 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 15 ++-
 drivers/scsi/qla2xxx/qla_os.c| 21 +
 2 files changed, 27 insertions(+), 9 deletions(-)

With full diff below.

James

---

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c 
b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 8aa769a..91b70bc 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -4010,7 +4010,10 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 
ioc_status)
SAM_STAT_CHECK_CONDITION;
 }
 
-
+static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
+{
+   return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
+}
 
 /**
  * scsih_qcmd - main scsi request entry point
@@ -4038,6 +4041,13 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
*scmd)
if (ioc->logging_level & MPT_DEBUG_SCSI)
scsi_print_command(scmd);
 
+   /*
+* Lock the device for any subsequent command until command is
+* done.
+*/
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_block(scmd->device);
+
sas_device_priv_data = scmd->device->hostdata;
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
scmd->result = DID_NO_CONNECT << 16;
@@ -4613,6 +4623,9 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 
msix_index, u32 reply)
if (scmd == NULL)
return 1;
 
+   if (ata_12_16_cmd(scmd))
+   scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
+
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
 
if (mpi_reply == NULL) {
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 567fa08..56d6142 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1456,15 +1456,20 @@ qla2x00_abort_all_cmds(scsi_qla_host_t *vha, int res)
for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
sp = req->outstanding_cmds[cnt];
if (sp) {
-   /* Get a reference to the sp and drop the lock.
-* The reference ensures this sp->done() call
-* - and not the call in qla2xxx_eh_abort() -
-* ends the SCSI command (with result 'res').
+   /* Don't abort commands in adapter during EEH
+* recovery as it's not accessible/responding.
 */
-   sp_get(sp);
-   spin_unlock_irqrestore(>hardware_lock, 
flags);
-   qla2xxx_eh_abort(GET_CMD_SP(sp));
-   spin_lock_irqsave(>hardware_lock, flags);
+   if (!ha->flags.eeh_busy) {
+   /* Get a reference to the sp and drop 
the lock.
+* The reference ensures this 
sp->done() call
+* - and not the call in 
qla2xxx_eh_abort() -
+* ends the SCSI command (with result 
'res').
+*/
+   sp_get(sp);
+   
spin_unlock_irqrestore(>hardware_lock, flags);
+   qla2xxx_eh_abort(GET_CMD_SP(sp));
+   spin_lock_irqsave(>hardware_lock, 
flags);
+   }
req->outstanding_cmds[cnt] = NULL;
sp->done(vha, sp, res);
}

Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver

[Sekhar Nori]

Hi Frank,

On Tuesday 22 November 2016 07:13 AM, Frank Rowand wrote:
> On 11/21/16 08:33, Sekhar Nori wrote:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> +   const struct da8xx_ddrctl_config_knob *knob;
>>> +   const struct da8xx_ddrctl_setting *setting;
>>> +   struct device_node *node;
>>> +   struct resource *res;
>>> +   void __iomem *ddrctl;
>>> +   struct device *dev;
>>> +   u32 reg;
>>> +
>>> +   dev = >dev;
>>> +   node = dev->of_node;
>>> +
>>> +   setting = da8xx_ddrctl_get_board_settings();
>>> +   if (!setting) {
>>> +   dev_err(dev, "no settings for board '%s'\n",
>>> +   of_flat_dt_get_machine_name());
>>> +   return -EINVAL;
>>> +   }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name() 
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting 
>> *da8xx_ddrctl_get_board_settings(void)
>>  return NULL;
>>  }
>>  
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> +const char *str;
>> +int ret;
>> +
>> +ret = of_property_read_string(of_root, "model", );
>> +if (ret)
>> +ret = of_property_read_string(of_root, "compatible", );
>> +
>> +return str;
>> +}
>> +
>>  static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>  {
>>  const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device 
>> *pdev)
>>  setting = da8xx_ddrctl_get_board_settings();
>>  if (!setting) {
>>  dev_err(dev, "no settings for board '%s'\n",
>> -of_flat_dt_get_machine_name());
> 
> da8xx_ddrctl_get_board_settings() tries to match based on the "compatible"
> property in the root node.  The "model" property in the root node has
> nothing to do with the failure to match. So creating and then using
> da8xx_ddrctl_get_machine_name() to potentially report model is not useful.
> 
> It should be sufficient to simply report that no compatible matched.

I agree with you on this. Even if model name is printed, you will have
to go back and check the compatible anyway. But I think it will be
useful to print the compatible instead of just reporting that nothing
matched.

Bartosz, if you agree too, could you send a fix patch just printing the
compatible?

Thanks,
Sekhar

Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver

[Sekhar Nori]

Hi Frank,

On Tuesday 22 November 2016 07:13 AM, Frank Rowand wrote:
> On 11/21/16 08:33, Sekhar Nori wrote:
>> On Monday 31 October 2016 08:15 PM, Bartosz Golaszewski wrote:
>>> +static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>> +{
>>> +   const struct da8xx_ddrctl_config_knob *knob;
>>> +   const struct da8xx_ddrctl_setting *setting;
>>> +   struct device_node *node;
>>> +   struct resource *res;
>>> +   void __iomem *ddrctl;
>>> +   struct device *dev;
>>> +   u32 reg;
>>> +
>>> +   dev = >dev;
>>> +   node = dev->of_node;
>>> +
>>> +   setting = da8xx_ddrctl_get_board_settings();
>>> +   if (!setting) {
>>> +   dev_err(dev, "no settings for board '%s'\n",
>>> +   of_flat_dt_get_machine_name());
>>> +   return -EINVAL;
>>> +   }
>>
>> This causes a section mismatch because of_flat_dt_get_machine_name() 
>> has an __init annotation. I did not notice that before, sorry.
>>
>> It can be fixed with a patch like below:
>>
>> ---8<---
>> diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
>> index a20e7bbbcbe0..9ca5aab3ac54 100644
>> --- a/drivers/memory/da8xx-ddrctl.c
>> +++ b/drivers/memory/da8xx-ddrctl.c
>> @@ -102,6 +102,18 @@ static const struct da8xx_ddrctl_setting 
>> *da8xx_ddrctl_get_board_settings(void)
>>  return NULL;
>>  }
>>  
>> +static const char* da8xx_ddrctl_get_machine_name(void)
>> +{
>> +const char *str;
>> +int ret;
>> +
>> +ret = of_property_read_string(of_root, "model", );
>> +if (ret)
>> +ret = of_property_read_string(of_root, "compatible", );
>> +
>> +return str;
>> +}
>> +
>>  static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>  {
>>  const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device 
>> *pdev)
>>  setting = da8xx_ddrctl_get_board_settings();
>>  if (!setting) {
>>  dev_err(dev, "no settings for board '%s'\n",
>> -of_flat_dt_get_machine_name());
> 
> da8xx_ddrctl_get_board_settings() tries to match based on the "compatible"
> property in the root node.  The "model" property in the root node has
> nothing to do with the failure to match. So creating and then using
> da8xx_ddrctl_get_machine_name() to potentially report model is not useful.
> 
> It should be sufficient to simply report that no compatible matched.

I agree with you on this. Even if model name is printed, you will have
to go back and check the compatible anyway. But I think it will be
useful to print the compatible instead of just reporting that nothing
matched.

Bartosz, if you agree too, could you send a fix patch just printing the
compatible?

Thanks,
Sekhar

Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed

[Dominik Brodowski]

On Mon, Nov 21, 2016 at 11:10:52PM +, David Howells wrote:
> One Thousand Gnomes  wrote:
> 
> > You need to filter or lock down kernel module options because a lot of
> > modules let you set the I/O port or similar (eg mmio) which means you can
> > hack the entire machine with say the 8250 driver just by using it with an
> > mmio of the right location to patch the secure state to zero just by
> > getting the ability to write to the modules conf file.
> 
> Is the attached patch the right sort of idea?  [Note that I haven't actually
> compiled most of these drivers to check my changes yet.]
> 
> David
> ---
> commit 8613a9655dad98c3358d82a9c4310cebdcb852ae
> Author: David Howells 
> Date:   Mon Nov 21 22:43:27 2016 +
> 
> Lock down drivers that can have io ports, io mem, irqs and dma changed
> 
> Lock down drivers that can have io ports, io mem, irqs and dma channels
> changed so that they can't be used to cause hardware to access the kernel
> image.
> 
> Notes:
> 
>  (1) module_isa_driver() gets an extra parameter that, if true, will cause
>  the module load to be rejected if the kernel is locked down.
> 
>  (2) module_driver() calls module_lockdown_check() to ask if the module
>  load should be rejected if the kernel is locked down.  This is a 
> macro
>  that should be #undef'd and then redefined right before
>  module_driver() is called.
> 
>  (3) module_pci_driver() is a wrapper around module_driver(), so the same
>  macro is used as in (2).
> 
>  (4) A number of drivers use parport 'ports' - so I haven't touched those.

You might also need to disable CIS overrides and CIS firmware loading for PCMCIA
drivers, I presume. That needs two changes:

- Abort in drivers/pcmcia/ds.c::pcmcia_load_firmware() or disable
  the CONFIG_PCMCIA_LOAD_CIS config option permanently.

- Abort in drivers/pcmcia/cistpl.c::pccard_store_cis() or remove
  write access to the "cis" file in
  drivers/pcmcia/cistpl.c::pccard_cis_attr

Best,
Dominik

Re: [PATCH] Lock down drivers that can have io ports, io mem, irqs and dma changed

[Dominik Brodowski]

On Mon, Nov 21, 2016 at 11:10:52PM +, David Howells wrote:
> One Thousand Gnomes  wrote:
> 
> > You need to filter or lock down kernel module options because a lot of
> > modules let you set the I/O port or similar (eg mmio) which means you can
> > hack the entire machine with say the 8250 driver just by using it with an
> > mmio of the right location to patch the secure state to zero just by
> > getting the ability to write to the modules conf file.
> 
> Is the attached patch the right sort of idea?  [Note that I haven't actually
> compiled most of these drivers to check my changes yet.]
> 
> David
> ---
> commit 8613a9655dad98c3358d82a9c4310cebdcb852ae
> Author: David Howells 
> Date:   Mon Nov 21 22:43:27 2016 +
> 
> Lock down drivers that can have io ports, io mem, irqs and dma changed
> 
> Lock down drivers that can have io ports, io mem, irqs and dma channels
> changed so that they can't be used to cause hardware to access the kernel
> image.
> 
> Notes:
> 
>  (1) module_isa_driver() gets an extra parameter that, if true, will cause
>  the module load to be rejected if the kernel is locked down.
> 
>  (2) module_driver() calls module_lockdown_check() to ask if the module
>  load should be rejected if the kernel is locked down.  This is a 
> macro
>  that should be #undef'd and then redefined right before
>  module_driver() is called.
> 
>  (3) module_pci_driver() is a wrapper around module_driver(), so the same
>  macro is used as in (2).
> 
>  (4) A number of drivers use parport 'ports' - so I haven't touched those.

You might also need to disable CIS overrides and CIS firmware loading for PCMCIA
drivers, I presume. That needs two changes:

- Abort in drivers/pcmcia/ds.c::pcmcia_load_firmware() or disable
  the CONFIG_PCMCIA_LOAD_CIS config option permanently.

- Abort in drivers/pcmcia/cistpl.c::pccard_store_cis() or remove
  write access to the "cis" file in
  drivers/pcmcia/cistpl.c::pccard_cis_attr

Best,
Dominik

Re: [PATCH v2 4/4] spi: spi-fsl-dspi: Minor code cleanup and error path fixes

[maitysanchayan]

On 16-11-21 15:22:09, Stefan Agner wrote:
> On 2016-11-20 21:54, Sanchayan Maity wrote:
> > Code cleanup for improving code readability and error path fixes
> > and cleanup removing use of devm_kfree.
> 
> Two things in one, not very nice. Especially the dma_free_coherent is
> really a bug and the other is a cleanup. Can you make a separate patch
> for the bug?
> 
> As for the cleanup, I don't like the one line conditions too, but I
> don't think it is worth a patch. At least the TX path should be solved
> with my suggestion in patch 2.

Agreed.

- Sanchayan.

> 
> --
> Stefan
> 
> > 
> > Signed-off-by: Sanchayan Maity 
> > ---
> >  drivers/spi/spi-fsl-dspi.c | 22 --
> >  1 file changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> > index 08882f7..2987a16 100644
> > --- a/drivers/spi/spi-fsl-dspi.c
> > +++ b/drivers/spi/spi-fsl-dspi.c
> > @@ -226,8 +226,10 @@ static void dspi_rx_dma_callback(void *arg)
> > if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
> > for (i = 0; i < dma->curr_xfer_len; i++) {
> > d = dspi->dma->rx_dma_buf[i];
> > -   rx_word ? (*(u16 *)dspi->rx = d) :
> > -   (*(u8 *)dspi->rx = d);
> > +   if (rx_word)
> > +   *(u16 *)dspi->rx = d;
> > +   else
> > +   *(u8 *)dspi->rx = d;
> > dspi->rx += rx_word + 1;
> > }
> > }
> > @@ -247,14 +249,20 @@ static int dspi_next_xfer_dma_submit(struct
> > fsl_dspi *dspi)
> > tx_word = is_double_byte_mode(dspi);
> >  
> > for (i = 0; i < dma->curr_xfer_len - 1; i++) {
> > -   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
> > +   if (tx_word)
> > +   val = *(u16 *) dspi->tx;
> > +   else
> > +   val = *(u8 *) dspi->tx;
> > dspi->dma->tx_dma_buf[i] =
> > SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
> > dspi->tx += tx_word + 1;
> > }
> >  
> > -   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
> > +   if (tx_word)
> > +   val = *(u16 *) dspi->tx;
> > +   else
> > +   val = *(u8 *) dspi->tx;
> > dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
> > SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0);
> > @@ -430,9 +438,11 @@ static int dspi_request_dma(struct fsl_dspi
> > *dspi, phys_addr_t phy_addr)
> > return 0;
> >  
> >  err_slave_config:
> > -   devm_kfree(dev, dma->rx_dma_buf);
> > +   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
> > +   dma->rx_dma_buf, dma->rx_dma_phys);
> >  err_rx_dma_buf:
> > -   devm_kfree(dev, dma->tx_dma_buf);
> > +   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
> > +   dma->tx_dma_buf, dma->tx_dma_phys);
> >  err_tx_dma_buf:
> > dma_release_channel(dma->chan_tx);
> >  err_tx_channel:

Re: [PATCH v2 4/4] spi: spi-fsl-dspi: Minor code cleanup and error path fixes

[maitysanchayan]

On 16-11-21 15:22:09, Stefan Agner wrote:
> On 2016-11-20 21:54, Sanchayan Maity wrote:
> > Code cleanup for improving code readability and error path fixes
> > and cleanup removing use of devm_kfree.
> 
> Two things in one, not very nice. Especially the dma_free_coherent is
> really a bug and the other is a cleanup. Can you make a separate patch
> for the bug?
> 
> As for the cleanup, I don't like the one line conditions too, but I
> don't think it is worth a patch. At least the TX path should be solved
> with my suggestion in patch 2.

Agreed.

- Sanchayan.

> 
> --
> Stefan
> 
> > 
> > Signed-off-by: Sanchayan Maity 
> > ---
> >  drivers/spi/spi-fsl-dspi.c | 22 --
> >  1 file changed, 16 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> > index 08882f7..2987a16 100644
> > --- a/drivers/spi/spi-fsl-dspi.c
> > +++ b/drivers/spi/spi-fsl-dspi.c
> > @@ -226,8 +226,10 @@ static void dspi_rx_dma_callback(void *arg)
> > if (!(dspi->dataflags & TRAN_STATE_RX_VOID)) {
> > for (i = 0; i < dma->curr_xfer_len; i++) {
> > d = dspi->dma->rx_dma_buf[i];
> > -   rx_word ? (*(u16 *)dspi->rx = d) :
> > -   (*(u8 *)dspi->rx = d);
> > +   if (rx_word)
> > +   *(u16 *)dspi->rx = d;
> > +   else
> > +   *(u8 *)dspi->rx = d;
> > dspi->rx += rx_word + 1;
> > }
> > }
> > @@ -247,14 +249,20 @@ static int dspi_next_xfer_dma_submit(struct
> > fsl_dspi *dspi)
> > tx_word = is_double_byte_mode(dspi);
> >  
> > for (i = 0; i < dma->curr_xfer_len - 1; i++) {
> > -   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
> > +   if (tx_word)
> > +   val = *(u16 *) dspi->tx;
> > +   else
> > +   val = *(u8 *) dspi->tx;
> > dspi->dma->tx_dma_buf[i] =
> > SPI_PUSHR_TXDATA(val) | SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0) | SPI_PUSHR_CONT;
> > dspi->tx += tx_word + 1;
> > }
> >  
> > -   val = tx_word ? *(u16 *) dspi->tx : *(u8 *) dspi->tx;
> > +   if (tx_word)
> > +   val = *(u16 *) dspi->tx;
> > +   else
> > +   val = *(u8 *) dspi->tx;
> > dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
> > SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0);
> > @@ -430,9 +438,11 @@ static int dspi_request_dma(struct fsl_dspi
> > *dspi, phys_addr_t phy_addr)
> > return 0;
> >  
> >  err_slave_config:
> > -   devm_kfree(dev, dma->rx_dma_buf);
> > +   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
> > +   dma->rx_dma_buf, dma->rx_dma_phys);
> >  err_rx_dma_buf:
> > -   devm_kfree(dev, dma->tx_dma_buf);
> > +   dma_free_coherent(dev, DSPI_DMA_BUFSIZE,
> > +   dma->tx_dma_buf, dma->tx_dma_phys);
> >  err_tx_dma_buf:
> > dma_release_channel(dma->chan_tx);
> >  err_tx_channel:

Re: [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format

[maitysanchayan]

On 16-11-21 15:15:41, Stefan Agner wrote:
> On 2016-11-20 21:54, Sanchayan Maity wrote:
> > Current DMA implementation was not handling the continuous selection
> > format viz. SPI chip select would be deasserted even between sequential
> > serial transfers. Use the cs_change variable and correctly set or
> > reset the CONT bit accordingly for case where peripherals require
> > the chip select to be asserted between sequential transfers.
> > 
> > Signed-off-by: Sanchayan Maity 
> > ---
> >  drivers/spi/spi-fsl-dspi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> > index b1ee1f5..41422cd 100644
> > --- a/drivers/spi/spi-fsl-dspi.c
> > +++ b/drivers/spi/spi-fsl-dspi.c
> > @@ -261,6 +261,8 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
> > *dspi)
> > dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
> > SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0);
> > +   if (!dspi->cs_change)
> > +   dspi->dma->tx_dma_buf[i] |= SPI_PUSHR_CONT;
> > dspi->tx += tx_word + 1;
> >  
> > dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
> 
> Other transfer mode use:
> 
> if ((dspi->cs_change) && (!dspi->len))  
>
> dspi_pushr &= ~SPI_PUSHR_CONT;
> 
> which indicates that they only clear SPI_PUSHR_CONT at the very end of a
> transfer... The DMA code currently deselects after every DMA transfer if
> dspi->cs_change is set.
> 
> Maybe we should use the helper dspi_data_to_pushr to fill the DMA buffer
> and _clear_ SPI_PUSHR_CONT if necessary like the other transfer modes
> do... Then we can use the for loop to fill the complete buffer and get
> rid of some code dupplication.
> 
> I see that dspi_data_to_pushr does move len too, which we did not in the
> DMA case. dspi->len gets incremented only on successful DMA transfer in
> dspi_dma_xfer. However, I wonder if that is not even a bug: We increment
> dspi->tx always, but len only on success. This makes len go off sync
> with regards to the tx pointer which does not help anybody. So lets get
> rid of the update code in dspi_dma_xfer
> 

Thanks for the feedback. Using dspi_data_to_pushr really cleans up that
tx path very nicely. Why didn't I see it. Will send a follow up patch
soon after testing again.

- Sanchayan.

Re: [PATCH v2 2/4] spi: spi-fsl-dspi: Fix continuous selection format

[maitysanchayan]

On 16-11-21 15:15:41, Stefan Agner wrote:
> On 2016-11-20 21:54, Sanchayan Maity wrote:
> > Current DMA implementation was not handling the continuous selection
> > format viz. SPI chip select would be deasserted even between sequential
> > serial transfers. Use the cs_change variable and correctly set or
> > reset the CONT bit accordingly for case where peripherals require
> > the chip select to be asserted between sequential transfers.
> > 
> > Signed-off-by: Sanchayan Maity 
> > ---
> >  drivers/spi/spi-fsl-dspi.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> > index b1ee1f5..41422cd 100644
> > --- a/drivers/spi/spi-fsl-dspi.c
> > +++ b/drivers/spi/spi-fsl-dspi.c
> > @@ -261,6 +261,8 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi 
> > *dspi)
> > dspi->dma->tx_dma_buf[i] = SPI_PUSHR_TXDATA(val) |
> > SPI_PUSHR_PCS(dspi->cs) |
> > SPI_PUSHR_CTAS(0);
> > +   if (!dspi->cs_change)
> > +   dspi->dma->tx_dma_buf[i] |= SPI_PUSHR_CONT;
> > dspi->tx += tx_word + 1;
> >  
> > dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
> 
> Other transfer mode use:
> 
> if ((dspi->cs_change) && (!dspi->len))  
>
> dspi_pushr &= ~SPI_PUSHR_CONT;
> 
> which indicates that they only clear SPI_PUSHR_CONT at the very end of a
> transfer... The DMA code currently deselects after every DMA transfer if
> dspi->cs_change is set.
> 
> Maybe we should use the helper dspi_data_to_pushr to fill the DMA buffer
> and _clear_ SPI_PUSHR_CONT if necessary like the other transfer modes
> do... Then we can use the for loop to fill the complete buffer and get
> rid of some code dupplication.
> 
> I see that dspi_data_to_pushr does move len too, which we did not in the
> DMA case. dspi->len gets incremented only on successful DMA transfer in
> dspi_dma_xfer. However, I wonder if that is not even a bug: We increment
> dspi->tx always, but len only on success. This makes len go off sync
> with regards to the tx pointer which does not help anybody. So lets get
> rid of the update code in dspi_dma_xfer
> 

Thanks for the feedback. Using dspi_data_to_pushr really cleans up that
tx path very nicely. Why didn't I see it. Will send a follow up patch
soon after testing again.

- Sanchayan.

Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE.

[Dave Young]

Hi Michael
On 11/22/16 at 05:01pm, Michael Ellerman wrote:
> Thiago Jung Bauermann  writes:
> > Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young:
> >> On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote:
> >> > powerpc's purgatory.ro has 12 relocation types when built as
> >> > a relocatable object. To implement support for them requires
> >> > arch_kexec_apply_relocations_add to duplicate a lot of code with
> >> > module_64.c:apply_relocate_add.
> >> > 
> >> > When built as a Position Independent Executable there are only 4
> >> > relocation types in purgatory.ro, so it becomes practical for the powerpc
> >> > implementation of kexec_file to have its own relocation implementation.
> >> > 
> >> > Also, the purgatory is an executable and not an intermediary output from
> >> > the compiler so it makes sense conceptually that it is easier to build
> >> > it as a PIE than as a partially linked object.
> >> > 
> >> > Apart from the greatly reduced number of relocations, there are two
> >> > differences between a relocatable object and a PIE:
> >> > 
> >> > 1. __kexec_load_purgatory needs to use the program headers rather than 
> >> > the
> >> > 
> >> >section headers to figure out how to load the binary.
> >> > 
> >> > 2. Symbol values are absolute addresses instead of relative to the
> >> > 
> >> >start of the section.
> >> > 
> >> > This patch adds the support needed in generic code for the differences
> >> > above and allows powerpc to load and relocate a position independent
> >> > purgatory.
> >> 
> >> [snip]
> >> 
> >> The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is
> >> not that complex. So could you look into simplify your kexec_file
> >> implementation?
> >
> > I can try, but there is one fundamental issue here: powerpc 
> > position-dependent 
> > code relies more on relocations than x86 position-dependent code does, so 
> > there's a limit to how simple it can be made without switching to position-
> > independent code. And it will always be more involved than it is on x86.
> 
> I think we need to go back to the drawing board on this one.
> 
> My hope was that building purgatory as PIE would reduce the amount of
> complexity, but instead it's just added more. Sorry for sending you in
> that direction.
> 
> 
> In general I dislike the level of complexity of the kexec-tools
> purgatory, and in particular I'm not comfortable with things like:
> 
> diff --git a/arch/powerpc/purgatory/sha256.c b/arch/powerpc/purgatory/sha256.c
> new file mode 100644
> index ..6abee1877d56
> --- /dev/null
> +++ b/arch/powerpc/purgatory/sha256.c
> @@ -0,0 +1,6 @@
> +#include "../boot/string.h"
> +
> +/* Avoid including x86's boot/string.h in sha256.c. */
> +#define BOOT_STRING_H
> +
> +#include "../../x86/purgatory/sha256.c"
> 

Agreed, include x86 code in powerpc looks bad

> 
> I think the best way to get this over the line would be to take the
> kexec-lite purgatory implementation and use that to begin with. I know
> it doesn't have all the features of the kexec-tools version, but it
> should work, and we can look at adding the extra features later.

Instead of adding other implementation, moving the purgatory sha256 code
out of x86 sounds better so that we can reuse them cleanly..

> 
> I'll try and get that working tonight.
> 
> cheers

Thanks
Dave

Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE.

[Dave Young]

Hi Michael
On 11/22/16 at 05:01pm, Michael Ellerman wrote:
> Thiago Jung Bauermann  writes:
> > Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young:
> >> On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote:
> >> > powerpc's purgatory.ro has 12 relocation types when built as
> >> > a relocatable object. To implement support for them requires
> >> > arch_kexec_apply_relocations_add to duplicate a lot of code with
> >> > module_64.c:apply_relocate_add.
> >> > 
> >> > When built as a Position Independent Executable there are only 4
> >> > relocation types in purgatory.ro, so it becomes practical for the powerpc
> >> > implementation of kexec_file to have its own relocation implementation.
> >> > 
> >> > Also, the purgatory is an executable and not an intermediary output from
> >> > the compiler so it makes sense conceptually that it is easier to build
> >> > it as a PIE than as a partially linked object.
> >> > 
> >> > Apart from the greatly reduced number of relocations, there are two
> >> > differences between a relocatable object and a PIE:
> >> > 
> >> > 1. __kexec_load_purgatory needs to use the program headers rather than 
> >> > the
> >> > 
> >> >section headers to figure out how to load the binary.
> >> > 
> >> > 2. Symbol values are absolute addresses instead of relative to the
> >> > 
> >> >start of the section.
> >> > 
> >> > This patch adds the support needed in generic code for the differences
> >> > above and allows powerpc to load and relocate a position independent
> >> > purgatory.
> >> 
> >> [snip]
> >> 
> >> The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is
> >> not that complex. So could you look into simplify your kexec_file
> >> implementation?
> >
> > I can try, but there is one fundamental issue here: powerpc 
> > position-dependent 
> > code relies more on relocations than x86 position-dependent code does, so 
> > there's a limit to how simple it can be made without switching to position-
> > independent code. And it will always be more involved than it is on x86.
> 
> I think we need to go back to the drawing board on this one.
> 
> My hope was that building purgatory as PIE would reduce the amount of
> complexity, but instead it's just added more. Sorry for sending you in
> that direction.
> 
> 
> In general I dislike the level of complexity of the kexec-tools
> purgatory, and in particular I'm not comfortable with things like:
> 
> diff --git a/arch/powerpc/purgatory/sha256.c b/arch/powerpc/purgatory/sha256.c
> new file mode 100644
> index ..6abee1877d56
> --- /dev/null
> +++ b/arch/powerpc/purgatory/sha256.c
> @@ -0,0 +1,6 @@
> +#include "../boot/string.h"
> +
> +/* Avoid including x86's boot/string.h in sha256.c. */
> +#define BOOT_STRING_H
> +
> +#include "../../x86/purgatory/sha256.c"
> 

Agreed, include x86 code in powerpc looks bad

> 
> I think the best way to get this over the line would be to take the
> kexec-lite purgatory implementation and use that to begin with. I know
> it doesn't have all the features of the kexec-tools version, but it
> should work, and we can look at adding the extra features later.

Instead of adding other implementation, moving the purgatory sha256 code
out of x86 sounds better so that we can reuse them cleanly..

> 
> I'll try and get that working tonight.
> 
> cheers

Thanks
Dave

Re: [PATCH 00/36] cputime: Convert core use of cputime_t to nsecs

[Martin Schwidefsky]

On Mon, 21 Nov 2016 17:20:06 +0100
Frederic Weisbecker  wrote:

> On Mon, Nov 21, 2016 at 07:59:56AM +0100, Martin Schwidefsky wrote:
> > On Fri, 18 Nov 2016 15:47:02 +0100
> > Frederic Weisbecker  wrote:
> > > > The do_account_vtime function is called once per jiffy and once per task
> > > > switch. HZ is usually set to 100 for s390, the conversion once per jiffy
> > > > would not be so bad, but the call on the scheduling path *will* hurt.
> > > 
> > > I don't think we need to flush on task switch. If we maintain the 
> > > accumulators
> > > on the task/thread struct instead of per-cpu, then the remaining time 
> > > after
> > > task switch out will be accounted on next tick after after next task 
> > > switch in.
> > 
> > You can not properly calculate steal time if you allow sleeping tasks to 
> > sit on
> > up to 5*HZ worth of cpu time.
> 
> Ah, you mean that when the task goes to sleep, we shouldn't miss more than one
> tick worth of system/user time but the steal time can be much higher, right?

No, it is worse than that. Consider a task going to sleep just before a tick
arrives. It will have almost a full HZ time-slice in its task specific 
accounting
numbers. After the switch another task with a different set of accounting 
numbers
is running. The tick will not push the cputime for the work done in the last
HZ period. Dependent on what the new task has in its accounting number the steal
time calculation can give you anything. Repeat the whole thing with any number
of tasks and the missing cputime can get really large. Now get one of these
processes back at the beginning of a time slice and you can get nearly 200% 
worth
of cputime in one tick. Switch to the next task with missing cputime at the 
start
of the new tick and you can get many ticks with too much cputime.

Not doing accounting on task switch is just broken.

> > I think we *have* to do accounting on task switch.
> > At least on s390, likely on powerpc as well. Why not make that an option for
> > the architecture with the yet-to-be-written accumulating code.
> 
> Ok, how about doing the accumulation and always account on task switch for 
> now,
> we'll see later if it's worth having such an option.

I am convinced that we need it. The prototype patch does it for s390.

-- 
blue skies,
   Martin.

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

Re: [PATCH 00/36] cputime: Convert core use of cputime_t to nsecs

[Martin Schwidefsky]

On Mon, 21 Nov 2016 17:20:06 +0100
Frederic Weisbecker  wrote:

> On Mon, Nov 21, 2016 at 07:59:56AM +0100, Martin Schwidefsky wrote:
> > On Fri, 18 Nov 2016 15:47:02 +0100
> > Frederic Weisbecker  wrote:
> > > > The do_account_vtime function is called once per jiffy and once per task
> > > > switch. HZ is usually set to 100 for s390, the conversion once per jiffy
> > > > would not be so bad, but the call on the scheduling path *will* hurt.
> > > 
> > > I don't think we need to flush on task switch. If we maintain the 
> > > accumulators
> > > on the task/thread struct instead of per-cpu, then the remaining time 
> > > after
> > > task switch out will be accounted on next tick after after next task 
> > > switch in.
> > 
> > You can not properly calculate steal time if you allow sleeping tasks to 
> > sit on
> > up to 5*HZ worth of cpu time.
> 
> Ah, you mean that when the task goes to sleep, we shouldn't miss more than one
> tick worth of system/user time but the steal time can be much higher, right?

No, it is worse than that. Consider a task going to sleep just before a tick
arrives. It will have almost a full HZ time-slice in its task specific 
accounting
numbers. After the switch another task with a different set of accounting 
numbers
is running. The tick will not push the cputime for the work done in the last
HZ period. Dependent on what the new task has in its accounting number the steal
time calculation can give you anything. Repeat the whole thing with any number
of tasks and the missing cputime can get really large. Now get one of these
processes back at the beginning of a time slice and you can get nearly 200% 
worth
of cputime in one tick. Switch to the next task with missing cputime at the 
start
of the new tick and you can get many ticks with too much cputime.

Not doing accounting on task switch is just broken.

> > I think we *have* to do accounting on task switch.
> > At least on s390, likely on powerpc as well. Why not make that an option for
> > the architecture with the yet-to-be-written accumulating code.
> 
> Ok, how about doing the accumulation and always account on task switch for 
> now,
> we'll see later if it's worth having such an option.

I am convinced that we need it. The prototype patch does it for s390.

-- 
blue skies,
   Martin.

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

[GIT PULL] extcon next for v4.10

[Chanwoo Choi]

Dear Greg,

This is extcon-next pull request for v4.10. I add detailed description of
this pull request on below. Please pull extcon with following updates.

Best Regards,
Chanwoo Choi

The following changes since commit a25f0944ba9b1d8a6813fd6f1a86f1bd59ac25a6:

  Linux 4.9-rc5 (2016-11-13 10:32:32 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git 
tags/extcon-next-for-4.10

for you to fetch changes up to 541332a13b1ded42097ba96c52c7bc70931e528c:

  extcon: usb-gpio: Add VBUS detection support (2016-11-20 21:22:39 +0900)


Update extcon for 4.10

Detailed description for this pull request:
- The extcon-usb-gpio driver supports the VBUS detection with USB ID and VBUS 
pin.


Roger Quadros (1):
  extcon: usb-gpio: Add VBUS detection support

 .../devicetree/bindings/extcon/extcon-usb-gpio.txt |   3 +
 drivers/extcon/extcon-usb-gpio.c   | 169 -
 2 files changed, 132 insertions(+), 40 deletions(-)

[GIT PULL] extcon next for v4.10

[Chanwoo Choi]

Dear Greg,

This is extcon-next pull request for v4.10. I add detailed description of
this pull request on below. Please pull extcon with following updates.

Best Regards,
Chanwoo Choi

The following changes since commit a25f0944ba9b1d8a6813fd6f1a86f1bd59ac25a6:

  Linux 4.9-rc5 (2016-11-13 10:32:32 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git 
tags/extcon-next-for-4.10

for you to fetch changes up to 541332a13b1ded42097ba96c52c7bc70931e528c:

  extcon: usb-gpio: Add VBUS detection support (2016-11-20 21:22:39 +0900)


Update extcon for 4.10

Detailed description for this pull request:
- The extcon-usb-gpio driver supports the VBUS detection with USB ID and VBUS 
pin.


Roger Quadros (1):
  extcon: usb-gpio: Add VBUS detection support

 .../devicetree/bindings/extcon/extcon-usb-gpio.txt |   3 +
 drivers/extcon/extcon-usb-gpio.c   | 169 -
 2 files changed, 132 insertions(+), 40 deletions(-)

Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE.

[Michael Ellerman]

Thiago Jung Bauermann  writes:
> Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young:
>> On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote:
>> > powerpc's purgatory.ro has 12 relocation types when built as
>> > a relocatable object. To implement support for them requires
>> > arch_kexec_apply_relocations_add to duplicate a lot of code with
>> > module_64.c:apply_relocate_add.
>> > 
>> > When built as a Position Independent Executable there are only 4
>> > relocation types in purgatory.ro, so it becomes practical for the powerpc
>> > implementation of kexec_file to have its own relocation implementation.
>> > 
>> > Also, the purgatory is an executable and not an intermediary output from
>> > the compiler so it makes sense conceptually that it is easier to build
>> > it as a PIE than as a partially linked object.
>> > 
>> > Apart from the greatly reduced number of relocations, there are two
>> > differences between a relocatable object and a PIE:
>> > 
>> > 1. __kexec_load_purgatory needs to use the program headers rather than the
>> > 
>> >section headers to figure out how to load the binary.
>> > 
>> > 2. Symbol values are absolute addresses instead of relative to the
>> > 
>> >start of the section.
>> > 
>> > This patch adds the support needed in generic code for the differences
>> > above and allows powerpc to load and relocate a position independent
>> > purgatory.
>> 
>> [snip]
>> 
>> The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is
>> not that complex. So could you look into simplify your kexec_file
>> implementation?
>
> I can try, but there is one fundamental issue here: powerpc 
> position-dependent 
> code relies more on relocations than x86 position-dependent code does, so 
> there's a limit to how simple it can be made without switching to position-
> independent code. And it will always be more involved than it is on x86.

I think we need to go back to the drawing board on this one.

My hope was that building purgatory as PIE would reduce the amount of
complexity, but instead it's just added more. Sorry for sending you in
that direction.


In general I dislike the level of complexity of the kexec-tools
purgatory, and in particular I'm not comfortable with things like:

diff --git a/arch/powerpc/purgatory/sha256.c b/arch/powerpc/purgatory/sha256.c
new file mode 100644
index ..6abee1877d56
--- /dev/null
+++ b/arch/powerpc/purgatory/sha256.c
@@ -0,0 +1,6 @@
+#include "../boot/string.h"
+
+/* Avoid including x86's boot/string.h in sha256.c. */
+#define BOOT_STRING_H
+
+#include "../../x86/purgatory/sha256.c"


I think the best way to get this over the line would be to take the
kexec-lite purgatory implementation and use that to begin with. I know
it doesn't have all the features of the kexec-tools version, but it
should work, and we can look at adding the extra features later.

I'll try and get that working tonight.

cheers

Re: [PATCH v10 04/10] kexec_file: Add support for purgatory built as PIE.

[Michael Ellerman]

Thiago Jung Bauermann  writes:
> Am Sonntag, 20. November 2016, 10:45:46 BRST schrieb Dave Young:
>> On 11/10/16 at 01:27am, Thiago Jung Bauermann wrote:
>> > powerpc's purgatory.ro has 12 relocation types when built as
>> > a relocatable object. To implement support for them requires
>> > arch_kexec_apply_relocations_add to duplicate a lot of code with
>> > module_64.c:apply_relocate_add.
>> > 
>> > When built as a Position Independent Executable there are only 4
>> > relocation types in purgatory.ro, so it becomes practical for the powerpc
>> > implementation of kexec_file to have its own relocation implementation.
>> > 
>> > Also, the purgatory is an executable and not an intermediary output from
>> > the compiler so it makes sense conceptually that it is easier to build
>> > it as a PIE than as a partially linked object.
>> > 
>> > Apart from the greatly reduced number of relocations, there are two
>> > differences between a relocatable object and a PIE:
>> > 
>> > 1. __kexec_load_purgatory needs to use the program headers rather than the
>> > 
>> >section headers to figure out how to load the binary.
>> > 
>> > 2. Symbol values are absolute addresses instead of relative to the
>> > 
>> >start of the section.
>> > 
>> > This patch adds the support needed in generic code for the differences
>> > above and allows powerpc to load and relocate a position independent
>> > purgatory.
>> 
>> [snip]
>> 
>> The kexec-tools machine_apply_elf_rel is pretty simple for ppc64, it is
>> not that complex. So could you look into simplify your kexec_file
>> implementation?
>
> I can try, but there is one fundamental issue here: powerpc 
> position-dependent 
> code relies more on relocations than x86 position-dependent code does, so 
> there's a limit to how simple it can be made without switching to position-
> independent code. And it will always be more involved than it is on x86.

I think we need to go back to the drawing board on this one.

My hope was that building purgatory as PIE would reduce the amount of
complexity, but instead it's just added more. Sorry for sending you in
that direction.


In general I dislike the level of complexity of the kexec-tools
purgatory, and in particular I'm not comfortable with things like:

diff --git a/arch/powerpc/purgatory/sha256.c b/arch/powerpc/purgatory/sha256.c
new file mode 100644
index ..6abee1877d56
--- /dev/null
+++ b/arch/powerpc/purgatory/sha256.c
@@ -0,0 +1,6 @@
+#include "../boot/string.h"
+
+/* Avoid including x86's boot/string.h in sha256.c. */
+#define BOOT_STRING_H
+
+#include "../../x86/purgatory/sha256.c"


I think the best way to get this over the line would be to take the
kexec-lite purgatory implementation and use that to begin with. I know
it doesn't have all the features of the kexec-tools version, but it
should work, and we can look at adding the extra features later.

I'll try and get that working tonight.

cheers

[PATCH 2/2] virtio_ring: fix complaint by sparse

[Gonglei]

 # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/

drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
(different base types)
drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
[usertype] nextflag
drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 degrades to 
integer

Signed-off-by: Gonglei 
---
 drivers/virtio/virtio_ring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 489bfc6..d2863c3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -420,7 +420,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
if (i == err_idx)
break;
vring_unmap_one(vq, [i]);
-   i = vq->vring.desc[i].next;
+   i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
}
 
vq->vq.num_free += total_sg;
@@ -601,7 +601,7 @@ bool virtqueue_kick(struct virtqueue *vq)
 static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
 {
unsigned int i, j;
-   u16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
+   __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
 
/* Clear data ptr. */
vq->desc_state[head].data = NULL;
-- 
1.8.3.1

[PATCH 2/2] virtio_ring: fix complaint by sparse

[Gonglei]

 # make C=2 CF="-D__CHECK_ENDIAN__" ./drivers/virtio/

drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:423:19: warning: incorrect type in assignment 
(different base types)
drivers/virtio/virtio_ring.c:423:19:expected unsigned int [unsigned] 
[assigned] i
drivers/virtio/virtio_ring.c:423:19:got restricted __virtio16 [usertype] 
next
drivers/virtio/virtio_ring.c:604:39: warning: incorrect type in initializer 
(different base types)
drivers/virtio/virtio_ring.c:604:39:expected unsigned short [unsigned] 
[usertype] nextflag
drivers/virtio/virtio_ring.c:604:39:got restricted __virtio16
drivers/virtio/virtio_ring.c:612:33: warning: restricted __virtio16 degrades to 
integer

Signed-off-by: Gonglei 
---
 drivers/virtio/virtio_ring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 489bfc6..d2863c3 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -420,7 +420,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
if (i == err_idx)
break;
vring_unmap_one(vq, [i]);
-   i = vq->vring.desc[i].next;
+   i = virtio16_to_cpu(_vq->vdev, vq->vring.desc[i].next);
}
 
vq->vq.num_free += total_sg;
@@ -601,7 +601,7 @@ bool virtqueue_kick(struct virtqueue *vq)
 static void detach_buf(struct vring_virtqueue *vq, unsigned int head)
 {
unsigned int i, j;
-   u16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
+   __virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
 
/* Clear data ptr. */
vq->desc_state[head].data = NULL;
-- 
1.8.3.1

Re: [PATCH] zram: Fix unbalanced idr management at hot removal

[Takashi Iwai]

On Tue, 22 Nov 2016 02:39:13 +0100,
Sergey Senozhatsky wrote:
> 
> On (11/22/16 10:33), Minchan Kim wrote:
> > On Tue, Nov 22, 2016 at 10:28:12AM +0900, Sergey Senozhatsky wrote:
> > > On (11/22/16 10:22), Minchan Kim wrote:
> > > [..]
> > > > > > Acked-by: Minchan Kim 
> > > > > 
> > > > > Link: lkml.kernel.org/r/20161121132140.12683-1-ti...@suse.de
> > > > > Fixes: 17ec4cd98578 ("zram: don't call idr_remove() from 
> > > > > zram_remove()")
> > > > > Cc: [4.5+]
> > > > 
> > > > 17ec4cd98578 marked as stable: [4.2+] so we should mark this patch as
> > > > 4.2+.
> > > 
> > > well, no objections. except that the patch in question was
> > > not picked up -stable
> > > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.2#n1365
> > > 
> > > the change appears starting from 4.5
> > > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.5#n1371
> 
> 
> > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.4.34#n1370
> 
> oh, ok. good find. 4.4+ then? there are no affected stable/longterm
> kernels older than 4.4.x anyway (4.1 lts has no dynamic zram management).

Right, at least, 4.4.x hits the issue (actually the original bug
report was for 4.4.x).


thanks,

Takashi

[PATCH 1/2] virtio_pci_modern: fix complaint by sparse

[Gonglei]

drivers/virtio/virtio_pci_modern.c:66:40: warning: incorrect type in argument 2 
(different base types)
drivers/virtio/virtio_pci_modern.c:66:40:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:66:40:got restricted __le32 [noderef] 
[usertype] *lo
drivers/virtio/virtio_pci_modern.c:67:33: warning: incorrect type in argument 2 
(different base types)
drivers/virtio/virtio_pci_modern.c:67:33:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:67:33:got restricted __le32 [noderef] 
[usertype] *hi
drivers/virtio/virtio_pci_modern.c:150:32: warning: incorrect type in argument 
2 (different base types)
drivers/virtio/virtio_pci_modern.c:150:32:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:150:32:got restricted __le32 [noderef] 
*
drivers/virtio/virtio_pci_modern.c:151:39: warning: incorrect type in argument 
1 (different base types)
drivers/virtio/virtio_pci_modern.c:151:39:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:151:39:got restricted __le32 [noderef] 
*
drivers/virtio/virtio_pci_modern.c:152:32: warning: incorrect type in argument 
2 (different base types)
drivers/virtio/virtio_pci_modern.c:152:32:expected unsigned int [noderef] 
[usertype] *addr

Signed-off-by: Gonglei 
---
 drivers/virtio/virtio_pci_modern.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index e76bd91..4bf7ab3 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -33,12 +33,12 @@ static inline u8 vp_ioread8(u8 __iomem *addr)
 {
return ioread8(addr);
 }
-static inline u16 vp_ioread16 (u16 __iomem *addr)
+static inline u16 vp_ioread16 (__le16 __iomem *addr)
 {
return ioread16(addr);
 }
 
-static inline u32 vp_ioread32(u32 __iomem *addr)
+static inline u32 vp_ioread32(__le32 __iomem *addr)
 {
return ioread32(addr);
 }
@@ -48,12 +48,12 @@ static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
iowrite8(value, addr);
 }
 
-static inline void vp_iowrite16(u16 value, u16 __iomem *addr)
+static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
 {
iowrite16(value, addr);
 }
 
-static inline void vp_iowrite32(u32 value, u32 __iomem *addr)
+static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
 {
iowrite32(value, addr);
 }
-- 
1.8.3.1

Re: [PATCH] zram: Fix unbalanced idr management at hot removal

[Takashi Iwai]

On Tue, 22 Nov 2016 02:39:13 +0100,
Sergey Senozhatsky wrote:
> 
> On (11/22/16 10:33), Minchan Kim wrote:
> > On Tue, Nov 22, 2016 at 10:28:12AM +0900, Sergey Senozhatsky wrote:
> > > On (11/22/16 10:22), Minchan Kim wrote:
> > > [..]
> > > > > > Acked-by: Minchan Kim 
> > > > > 
> > > > > Link: lkml.kernel.org/r/20161121132140.12683-1-ti...@suse.de
> > > > > Fixes: 17ec4cd98578 ("zram: don't call idr_remove() from 
> > > > > zram_remove()")
> > > > > Cc: [4.5+]
> > > > 
> > > > 17ec4cd98578 marked as stable: [4.2+] so we should mark this patch as
> > > > 4.2+.
> > > 
> > > well, no objections. except that the patch in question was
> > > not picked up -stable
> > > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.2#n1365
> > > 
> > > the change appears starting from 4.5
> > > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.5#n1371
> 
> 
> > https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/block/zram/zram_drv.c?id=refs/tags/v4.4.34#n1370
> 
> oh, ok. good find. 4.4+ then? there are no affected stable/longterm
> kernels older than 4.4.x anyway (4.1 lts has no dynamic zram management).

Right, at least, 4.4.x hits the issue (actually the original bug
report was for 4.4.x).


thanks,

Takashi

[PATCH 1/2] virtio_pci_modern: fix complaint by sparse

[Gonglei]

drivers/virtio/virtio_pci_modern.c:66:40: warning: incorrect type in argument 2 
(different base types)
drivers/virtio/virtio_pci_modern.c:66:40:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:66:40:got restricted __le32 [noderef] 
[usertype] *lo
drivers/virtio/virtio_pci_modern.c:67:33: warning: incorrect type in argument 2 
(different base types)
drivers/virtio/virtio_pci_modern.c:67:33:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:67:33:got restricted __le32 [noderef] 
[usertype] *hi
drivers/virtio/virtio_pci_modern.c:150:32: warning: incorrect type in argument 
2 (different base types)
drivers/virtio/virtio_pci_modern.c:150:32:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:150:32:got restricted __le32 [noderef] 
*
drivers/virtio/virtio_pci_modern.c:151:39: warning: incorrect type in argument 
1 (different base types)
drivers/virtio/virtio_pci_modern.c:151:39:expected unsigned int [noderef] 
[usertype] *addr
drivers/virtio/virtio_pci_modern.c:151:39:got restricted __le32 [noderef] 
*
drivers/virtio/virtio_pci_modern.c:152:32: warning: incorrect type in argument 
2 (different base types)
drivers/virtio/virtio_pci_modern.c:152:32:expected unsigned int [noderef] 
[usertype] *addr

Signed-off-by: Gonglei 
---
 drivers/virtio/virtio_pci_modern.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index e76bd91..4bf7ab3 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -33,12 +33,12 @@ static inline u8 vp_ioread8(u8 __iomem *addr)
 {
return ioread8(addr);
 }
-static inline u16 vp_ioread16 (u16 __iomem *addr)
+static inline u16 vp_ioread16 (__le16 __iomem *addr)
 {
return ioread16(addr);
 }
 
-static inline u32 vp_ioread32(u32 __iomem *addr)
+static inline u32 vp_ioread32(__le32 __iomem *addr)
 {
return ioread32(addr);
 }
@@ -48,12 +48,12 @@ static inline void vp_iowrite8(u8 value, u8 __iomem *addr)
iowrite8(value, addr);
 }
 
-static inline void vp_iowrite16(u16 value, u16 __iomem *addr)
+static inline void vp_iowrite16(u16 value, __le16 __iomem *addr)
 {
iowrite16(value, addr);
 }
 
-static inline void vp_iowrite32(u32 value, u32 __iomem *addr)
+static inline void vp_iowrite32(u32 value, __le32 __iomem *addr)
 {
iowrite32(value, addr);
 }
-- 
1.8.3.1

[PATCH 0/2] virtio: fix complaint by sparse

[Gonglei]

I found some warnings reported by sparse in the virtio code
when I checked virtio-crypto's driver stuff. Let's fix them.

Gonglei (2):
  virtio_pci_modern: fix complaint by sparse
  virtio_ring: fix complaint by sparse

 drivers/virtio/virtio_pci_modern.c | 8 
 drivers/virtio/virtio_ring.c   | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.8.3.1

[PATCH 0/2] virtio: fix complaint by sparse

[Gonglei]

I found some warnings reported by sparse in the virtio code
when I checked virtio-crypto's driver stuff. Let's fix them.

Gonglei (2):
  virtio_pci_modern: fix complaint by sparse
  virtio_ring: fix complaint by sparse

 drivers/virtio/virtio_pci_modern.c | 8 
 drivers/virtio/virtio_ring.c   | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.8.3.1

linux-next: manual merge of the tip tree with the pm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in:

  drivers/idle/Kconfig
  drivers/idle/i7300_idle.c

between commits:

  f2f31bb3f50b ("idle / i7300: Add CONFIG_PCI dependency")
  ccd29e2ed6bc ("i7300_idle: avoid theoretical uninitialized data usage")
  881ef1847b2d ("i7300_idle: fix typo in parameter description")

from the pm tree and commit:

  b65ce83f2a60 ("i7300_idle: Remove this driver")

from the tip tree.

I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

linux-next: manual merge of the tip tree with the pm tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the tip tree got a conflict in:

  drivers/idle/Kconfig
  drivers/idle/i7300_idle.c

between commits:

  f2f31bb3f50b ("idle / i7300: Add CONFIG_PCI dependency")
  ccd29e2ed6bc ("i7300_idle: avoid theoretical uninitialized data usage")
  881ef1847b2d ("i7300_idle: fix typo in parameter description")

from the pm tree and commit:

  b65ce83f2a60 ("i7300_idle: Remove this driver")

from the tip tree.

I fixed it up (I just used the latter version) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging.  You may also want to consider
cooperating with the maintainer of the conflicting tree to minimise any
particularly complex conflicts.

-- 
Cheers,
Stephen Rothwell

Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

[Anshuman Khandual]

On 11/21/2016 05:57 PM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 12:11:50PM +0530, Anshuman Khandual wrote:
>> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
> 
> [...]
> 
>>> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, 
>>> unsigned long end)
>>> remove_pagetable(start, end, true);
>>>  }
>>>  
>>> -int __ref arch_remove_memory(u64 start, u64 size)
>>> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
>>>  {
>>> unsigned long start_pfn = start >> PAGE_SHIFT;
>>> unsigned long nr_pages = size >> PAGE_SHIFT;
>>> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
>>> struct zone *zone;
>>> int ret;
>>>  
>>> +   /* Need to add support for device and unaddressable memory if needed */
>>> +   if (flags & MEMORY_UNADDRESSABLE) {
>>> +   BUG();
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> /* With altmap the first mapped page is offset from @start */
>>> altmap = to_vmem_altmap((unsigned long) page);
>>> if (altmap)
>>
>> So with this patch none of the architectures support un-addressable
>> memory but then support will be added through later patches ?
>> zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
>> Then we need to change all the previous ZONE_DEVICE changes which
>> ever took "for_device" to accommodate this new flag ? just curious.
> 
> Yes correct.
> 
> 
>>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>>> index 01033fa..ba9b12e 100644
>>> --- a/include/linux/memory_hotplug.h
>>> +++ b/include/linux/memory_hotplug.h
>>> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
>>>  
>>>  #ifdef CONFIG_MEMORY_HOTREMOVE
>>>  extern bool is_pageblock_removable_nolock(struct page *page);
>>> -extern int arch_remove_memory(u64 start, u64 size);
>>> +extern int arch_remove_memory(u64 start, u64 size, int flags);
>>>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
>>> unsigned long nr_pages);
>>>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>>> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
>>>  extern int add_memory_resource(int nid, struct resource *resource, bool 
>>> online);
>>>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
>>> bool for_device);
>>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
>>> +
>>> +/*
>>> + * For device memory we want more informations than just knowing it is 
>>> device
>>> + * memory. We want to know if we can migrate it (ie it is not storage 
>>> memory
>>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
>>> + * memory can not be access by CPU but we still want struct page so that we
>>> + * can use it like regular memory.
>>
>> Some typos here. Needs to be cleaned up as well. But please have a
>> look at comment below over the classification itself.
>>
>>> + */
>>> +#define MEMORY_FLAGS_NONE 0
>>> +#define MEMORY_DEVICE (1 << 0)
>>> +#define MEMORY_MOVABLE (1 << 1)
>>> +#define MEMORY_UNADDRESSABLE (1 << 2)
>>
>> It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
>> classify device memory (though they are represented with struct page)
>> not regular system ram memory. This should attempt to classify device
>> memory which is backed by struct pages. arch_add_memory/arch_remove
>> _memory does not come into play if it's traditional device memory
>> which is just PFN and does not have struct page associated with it.
> 
> Good idea i will change that.
> 
> 
>> Broadly they are either CPU accessible or in-accessible. Storage
>> memory like persistent memory represented though ZONE_DEVICE fall
>> under the accessible (coherent) category. IIUC right now they are
>> not movable because page->pgmap replaces page->lru in struct page
>> hence its inability to be on standard LRU lists as one of the
>> reasons. As there was a need to have struct page to exploit more
>> core VM features on these memory going forward it will have to be
>> migratable one way or the other to accommodate features like
>> compaction, HW poison etc in these storage memory. Hence my point
>> here is lets not classify any of these memories as non-movable.
>> Just addressable or not should be the only classification.
> 
> Being on the lru or not is not and issue in respect to migration. Being

Right, provided we we create separate migration interfaces for these non
LRU pages (preferably through HMM migration API layer). But where it
stands today, for NUMA migrate_pages() interface device non LRU memory
is a problem and we cannot use it for migration. Hence I brought up the
non LRU issue here.

> on the lru was use as an indication that the page is manage through the
> standard mm code and thus that many assumptions hold which in turn do
> allow migration. But if one use device memory following all rules of
> regular memory then migration can be done to no matter if page is on
> lru or not.

Right.

> 
> 

Re: [RFC PATCH net v2 2/3] dt: bindings: add ethernet phy eee-disable-advert option documentation

[Florian Fainelli]

Le 21/11/2016 à 08:47, Andrew Lunn a écrit :
>> What I did not realize when doing this patch for the realtek driver is
>> that there is already 6 valid modes defined in the kernel
>>
>> #define MDIO_EEE_100TX   MDIO_AN_EEE_ADV_100TX   /*
>> 100TX EEE cap */
>> #define MDIO_EEE_1000T   MDIO_AN_EEE_ADV_1000T   /*
>> 1000T EEE cap */
>> #define MDIO_EEE_10GT0x0008  /* 10GT EEE cap */
>> #define MDIO_EEE_1000KX  0x0010  /* 1000KX EEE cap
>> */
>> #define MDIO_EEE_10GKX4  0x0020  /* 10G KX4 EEE cap
>> */
>> #define MDIO_EEE_10GKR   0x0040  /* 10G KR EEE cap
>> */
>>
>> I took care of only 2 in the case of realtek.c since it only support
>> MDIO_EEE_100TX and MDIO_EEE_1000T.
>>
>> Defining a property for each is certainly doable but it does not look
>> very nice either. If it extends in the future, it will get even more
>> messier, especially if you want to disable everything.
> 
> Yes, agreed.

One risk with the definition a group of advertisement capabilities
(under the form of a bitmask for instance) to enable/disable is that we
end up with Device Tree contain some kind of configuration policy as
opposed to just flagging particular hardware features as broken.

Fortunately, there does not seem to be a ton of PHYs out there which
require EEE to be disabled to function properly so having individual
properties vs. bitmasks/groups is kind of speculative here.

Another approach to solving this problem could be to register a PHY
fixup which disables EEE at the PHY level, and which is only called for
specific boards affected by this problem (of_machine_is_compatible()).
This code can leave in arch/*/* when that is possible, or it can just be
somewhere where it is relevant, e.g; in the PHY driver for instance
(similarly to how PCI fixups are done).
-- 
Florian

Re: [HMM v13 01/18] mm/memory/hotplug: convert device parameter bool to set of flags

[Anshuman Khandual]

On 11/21/2016 05:57 PM, Jerome Glisse wrote:
> On Mon, Nov 21, 2016 at 12:11:50PM +0530, Anshuman Khandual wrote:
>> On 11/18/2016 11:48 PM, Jérôme Glisse wrote:
> 
> [...]
> 
>>> @@ -956,7 +963,7 @@ kernel_physical_mapping_remove(unsigned long start, 
>>> unsigned long end)
>>> remove_pagetable(start, end, true);
>>>  }
>>>  
>>> -int __ref arch_remove_memory(u64 start, u64 size)
>>> +int __ref arch_remove_memory(u64 start, u64 size, int flags)
>>>  {
>>> unsigned long start_pfn = start >> PAGE_SHIFT;
>>> unsigned long nr_pages = size >> PAGE_SHIFT;
>>> @@ -965,6 +972,12 @@ int __ref arch_remove_memory(u64 start, u64 size)
>>> struct zone *zone;
>>> int ret;
>>>  
>>> +   /* Need to add support for device and unaddressable memory if needed */
>>> +   if (flags & MEMORY_UNADDRESSABLE) {
>>> +   BUG();
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> /* With altmap the first mapped page is offset from @start */
>>> altmap = to_vmem_altmap((unsigned long) page);
>>> if (altmap)
>>
>> So with this patch none of the architectures support un-addressable
>> memory but then support will be added through later patches ?
>> zone_for_memory function's flag now takes MEMORY_DEVICE parameter.
>> Then we need to change all the previous ZONE_DEVICE changes which
>> ever took "for_device" to accommodate this new flag ? just curious.
> 
> Yes correct.
> 
> 
>>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>>> index 01033fa..ba9b12e 100644
>>> --- a/include/linux/memory_hotplug.h
>>> +++ b/include/linux/memory_hotplug.h
>>> @@ -103,7 +103,7 @@ extern bool memhp_auto_online;
>>>  
>>>  #ifdef CONFIG_MEMORY_HOTREMOVE
>>>  extern bool is_pageblock_removable_nolock(struct page *page);
>>> -extern int arch_remove_memory(u64 start, u64 size);
>>> +extern int arch_remove_memory(u64 start, u64 size, int flags);
>>>  extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
>>> unsigned long nr_pages);
>>>  #endif /* CONFIG_MEMORY_HOTREMOVE */
>>> @@ -275,7 +275,20 @@ extern int add_memory(int nid, u64 start, u64 size);
>>>  extern int add_memory_resource(int nid, struct resource *resource, bool 
>>> online);
>>>  extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default,
>>> bool for_device);
>>> -extern int arch_add_memory(int nid, u64 start, u64 size, bool for_device);
>>> +
>>> +/*
>>> + * For device memory we want more informations than just knowing it is 
>>> device
>>> + * memory. We want to know if we can migrate it (ie it is not storage 
>>> memory
>>> + * use by DAX). Is it addressable by the CPU ? Some device memory like GPU
>>> + * memory can not be access by CPU but we still want struct page so that we
>>> + * can use it like regular memory.
>>
>> Some typos here. Needs to be cleaned up as well. But please have a
>> look at comment below over the classification itself.
>>
>>> + */
>>> +#define MEMORY_FLAGS_NONE 0
>>> +#define MEMORY_DEVICE (1 << 0)
>>> +#define MEMORY_MOVABLE (1 << 1)
>>> +#define MEMORY_UNADDRESSABLE (1 << 2)
>>
>> It should be DEVICE_MEMORY_* instead of MEMORY_* as we are trying to
>> classify device memory (though they are represented with struct page)
>> not regular system ram memory. This should attempt to classify device
>> memory which is backed by struct pages. arch_add_memory/arch_remove
>> _memory does not come into play if it's traditional device memory
>> which is just PFN and does not have struct page associated with it.
> 
> Good idea i will change that.
> 
> 
>> Broadly they are either CPU accessible or in-accessible. Storage
>> memory like persistent memory represented though ZONE_DEVICE fall
>> under the accessible (coherent) category. IIUC right now they are
>> not movable because page->pgmap replaces page->lru in struct page
>> hence its inability to be on standard LRU lists as one of the
>> reasons. As there was a need to have struct page to exploit more
>> core VM features on these memory going forward it will have to be
>> migratable one way or the other to accommodate features like
>> compaction, HW poison etc in these storage memory. Hence my point
>> here is lets not classify any of these memories as non-movable.
>> Just addressable or not should be the only classification.
> 
> Being on the lru or not is not and issue in respect to migration. Being

Right, provided we we create separate migration interfaces for these non
LRU pages (preferably through HMM migration API layer). But where it
stands today, for NUMA migrate_pages() interface device non LRU memory
is a problem and we cannot use it for migration. Hence I brought up the
non LRU issue here.

> on the lru was use as an indication that the page is manage through the
> standard mm code and thus that many assumptions hold which in turn do
> allow migration. But if one use device memory following all rules of
> regular memory then migration can be done to no matter if page is on
> lru or not.

Right.

> 
> 

Re: [RFC PATCH net v2 2/3] dt: bindings: add ethernet phy eee-disable-advert option documentation

[Florian Fainelli]

Le 21/11/2016 à 08:47, Andrew Lunn a écrit :
>> What I did not realize when doing this patch for the realtek driver is
>> that there is already 6 valid modes defined in the kernel
>>
>> #define MDIO_EEE_100TX   MDIO_AN_EEE_ADV_100TX   /*
>> 100TX EEE cap */
>> #define MDIO_EEE_1000T   MDIO_AN_EEE_ADV_1000T   /*
>> 1000T EEE cap */
>> #define MDIO_EEE_10GT0x0008  /* 10GT EEE cap */
>> #define MDIO_EEE_1000KX  0x0010  /* 1000KX EEE cap
>> */
>> #define MDIO_EEE_10GKX4  0x0020  /* 10G KX4 EEE cap
>> */
>> #define MDIO_EEE_10GKR   0x0040  /* 10G KR EEE cap
>> */
>>
>> I took care of only 2 in the case of realtek.c since it only support
>> MDIO_EEE_100TX and MDIO_EEE_1000T.
>>
>> Defining a property for each is certainly doable but it does not look
>> very nice either. If it extends in the future, it will get even more
>> messier, especially if you want to disable everything.
> 
> Yes, agreed.

One risk with the definition a group of advertisement capabilities
(under the form of a bitmask for instance) to enable/disable is that we
end up with Device Tree contain some kind of configuration policy as
opposed to just flagging particular hardware features as broken.

Fortunately, there does not seem to be a ton of PHYs out there which
require EEE to be disabled to function properly so having individual
properties vs. bitmasks/groups is kind of speculative here.

Another approach to solving this problem could be to register a PHY
fixup which disables EEE at the PHY level, and which is only called for
specific boards affected by this problem (of_machine_is_compatible()).
This code can leave in arch/*/* when that is possible, or it can just be
somewhere where it is relevant, e.g; in the PHY driver for instance
(similarly to how PCI fixups are done).
-- 
Florian

[PATCH 1/1] ACPI: Document _OSI and _REV for Linux BIOS writers

[Len Brown]

From: Len Brown 

Based on a recent session at the Linux Plumber's Conference,
we need to be more clear about how a BIOS should use _OSI
to properly support Linux.

Signed-off-by: Len Brown 
---
 Documentation/acpi/osi.txt | 187 +
 1 file changed, 187 insertions(+)
 create mode 100644 Documentation/acpi/osi.txt

diff --git a/Documentation/acpi/osi.txt b/Documentation/acpi/osi.txt
new file mode 100644
index ..f9a0352f1d28
--- /dev/null
+++ b/Documentation/acpi/osi.txt
@@ -0,0 +1,187 @@
+ACPI _OSI and _REV methods
+--
+
+The ACPI _OSI method, "Operating System Interfaces",
+informs the ACPI BIOS about 'interfaces' supported by the Operating System.
+
+The ACPI _REV method returns the "Revision of the ACPI specification
+that OSPM supports"
+
+This document explains how and why the BIOS and Linux should use these methods.
+It also explains how and why they are widely mis-used.
+
+How to use _OSI
+---
+
+Linux runs on two groups of machines -- those that are tested by the OEM
+to be compatible with Linux, and those that were never tested with Linux,
+but where Linux was installed to replace the original OS (Windows or OSX).
+
+The larger group is the systems tested to run only Windows.
+Many of those systems were not only tested with just Windows, they were
+tested with just one specific version of Windows.  Even though the BIOS
+uses _OSI to query what version of Windows is running, only a single
+path through the BIOS has every been tested.
+For that reason, Linux must continue to claim compatibility with
+all versions of Windows.  To do otherwise exposes BIOS bugs on this
+large group of machines.
+
+But Linux isn't actually compatible with Windows, and the Linux community
+has also been hurt with regressions when Linux adds the latest version of
+Windows to its list of _OSI strings.  So it is possible that additional strings
+will be more thoroughly vetted before shipping upstream in the future.
+But it is likely that they will all eventually be added.
+
+Then there are systems designed and tested to run Linux.
+Often an OEM's ACPI BIOS team wants to support a unique feature
+in Linux, to do this, the BIOS should ask exactly what it wants to know:
+
+_OSI("Linux-OEM-my_interface_name")
+where 'OEM' is needed if this is an OEM-specific hook,
+and 'my_interface_name' describes the hook, which could be a
+quirk, a bug, or a bug-fix.
+
+For this to work, a patch must be sent to upstream Linux
+via the linux-a...@vger.kernel.org mailing list.  When that patch
+is checked into Linux, the OS will answer "YES" when the BIOS
+on the OEM's system uses _OSI to ask if the interface is supported
+by the OS.  Linux distributors can back-port that patch for Linux
+pre-installs, and it will be included by all distributions that
+re-base to upstream.  If the distribution can not update the kernel binary,
+they can also add an acpi_osi= cmdline parameter to the boot loader, as needed.
+
+If the string refers to a feature where the upstream kernel
+eventually grows support, a patch should be sent to remove
+the string when that support is added to the kernel.
+
+That was easy.  Read on, to find out how to do it wrong.
+
+Before _OSI, there was _OS
+--
+
+ACPI 1.0 specified "_OS" as an
+"object that evaluates to a string that identifies the operating system."
+
+The ACPI BIOS flow would include an evaluation of _OS, and the AML
+interpreter in the kernel would return to it a string identifying
+the OS.  _OS returned "Microsoft Windows" on Windows 98 and SE.
+It returned "Microsoft WindowsME:Millenium Edition" for Win-ME,
+and for Windows NT it returned "Microsoft Windows NT".  After that,
+_OS was abandoned, though modern BIOS continue to contain
+an ancient idiom looking for "Microsoft Windows NT" -- just
+in case time travel becomes possible?
+
+On a platform tasked with running multiple versions of Windows,
+the BIOS could use _OS to enable/disable devices that and OS
+might support, or enable/disable quirks or bug work-arounds
+necessary to make the platform compatible with a pre-existing OS.
+
+If a BIOS invokes _OS today, Linux will return
+"Microsoft Windows NT".  That is the *only* possible choice,
+as doing otherwise can steer the BIOS down an un-tested path.
+
+_OS had fundamental problems.  First, the BIOS needed to know the name
+of every version of the OS that would run on it, and needed to know
+all the quirks of those OS's.  Certainly it would make more sense
+for the BIOS to ask *specific* things of the OS, such
+"do you support a specific interface", and thus in ACPI 3.0,
+_OSI was born to replace _OS.
+
+_OSI is born, and immediately mis-used
+--
+
+With _OSI, the *BIOS* provides the string describing an interface,
+and asks the OS: "YES/NO, are you compatible with this interface?"
+
+eg. _OSI("3.0 Thermal Model") 

[PATCH 1/1] ACPI: Document _OSI and _REV for Linux BIOS writers

[Len Brown]

From: Len Brown 

Based on a recent session at the Linux Plumber's Conference,
we need to be more clear about how a BIOS should use _OSI
to properly support Linux.

Signed-off-by: Len Brown 
---
 Documentation/acpi/osi.txt | 187 +
 1 file changed, 187 insertions(+)
 create mode 100644 Documentation/acpi/osi.txt

diff --git a/Documentation/acpi/osi.txt b/Documentation/acpi/osi.txt
new file mode 100644
index ..f9a0352f1d28
--- /dev/null
+++ b/Documentation/acpi/osi.txt
@@ -0,0 +1,187 @@
+ACPI _OSI and _REV methods
+--
+
+The ACPI _OSI method, "Operating System Interfaces",
+informs the ACPI BIOS about 'interfaces' supported by the Operating System.
+
+The ACPI _REV method returns the "Revision of the ACPI specification
+that OSPM supports"
+
+This document explains how and why the BIOS and Linux should use these methods.
+It also explains how and why they are widely mis-used.
+
+How to use _OSI
+---
+
+Linux runs on two groups of machines -- those that are tested by the OEM
+to be compatible with Linux, and those that were never tested with Linux,
+but where Linux was installed to replace the original OS (Windows or OSX).
+
+The larger group is the systems tested to run only Windows.
+Many of those systems were not only tested with just Windows, they were
+tested with just one specific version of Windows.  Even though the BIOS
+uses _OSI to query what version of Windows is running, only a single
+path through the BIOS has every been tested.
+For that reason, Linux must continue to claim compatibility with
+all versions of Windows.  To do otherwise exposes BIOS bugs on this
+large group of machines.
+
+But Linux isn't actually compatible with Windows, and the Linux community
+has also been hurt with regressions when Linux adds the latest version of
+Windows to its list of _OSI strings.  So it is possible that additional strings
+will be more thoroughly vetted before shipping upstream in the future.
+But it is likely that they will all eventually be added.
+
+Then there are systems designed and tested to run Linux.
+Often an OEM's ACPI BIOS team wants to support a unique feature
+in Linux, to do this, the BIOS should ask exactly what it wants to know:
+
+_OSI("Linux-OEM-my_interface_name")
+where 'OEM' is needed if this is an OEM-specific hook,
+and 'my_interface_name' describes the hook, which could be a
+quirk, a bug, or a bug-fix.
+
+For this to work, a patch must be sent to upstream Linux
+via the linux-a...@vger.kernel.org mailing list.  When that patch
+is checked into Linux, the OS will answer "YES" when the BIOS
+on the OEM's system uses _OSI to ask if the interface is supported
+by the OS.  Linux distributors can back-port that patch for Linux
+pre-installs, and it will be included by all distributions that
+re-base to upstream.  If the distribution can not update the kernel binary,
+they can also add an acpi_osi= cmdline parameter to the boot loader, as needed.
+
+If the string refers to a feature where the upstream kernel
+eventually grows support, a patch should be sent to remove
+the string when that support is added to the kernel.
+
+That was easy.  Read on, to find out how to do it wrong.
+
+Before _OSI, there was _OS
+--
+
+ACPI 1.0 specified "_OS" as an
+"object that evaluates to a string that identifies the operating system."
+
+The ACPI BIOS flow would include an evaluation of _OS, and the AML
+interpreter in the kernel would return to it a string identifying
+the OS.  _OS returned "Microsoft Windows" on Windows 98 and SE.
+It returned "Microsoft WindowsME:Millenium Edition" for Win-ME,
+and for Windows NT it returned "Microsoft Windows NT".  After that,
+_OS was abandoned, though modern BIOS continue to contain
+an ancient idiom looking for "Microsoft Windows NT" -- just
+in case time travel becomes possible?
+
+On a platform tasked with running multiple versions of Windows,
+the BIOS could use _OS to enable/disable devices that and OS
+might support, or enable/disable quirks or bug work-arounds
+necessary to make the platform compatible with a pre-existing OS.
+
+If a BIOS invokes _OS today, Linux will return
+"Microsoft Windows NT".  That is the *only* possible choice,
+as doing otherwise can steer the BIOS down an un-tested path.
+
+_OS had fundamental problems.  First, the BIOS needed to know the name
+of every version of the OS that would run on it, and needed to know
+all the quirks of those OS's.  Certainly it would make more sense
+for the BIOS to ask *specific* things of the OS, such
+"do you support a specific interface", and thus in ACPI 3.0,
+_OSI was born to replace _OS.
+
+_OSI is born, and immediately mis-used
+--
+
+With _OSI, the *BIOS* provides the string describing an interface,
+and asks the OS: "YES/NO, are you compatible with this interface?"
+
+eg. _OSI("3.0 Thermal Model") would return TRUE if the OS knows how
+to 

linux-next: manual merge of the audit tree with Linus' tree

[Stephen Rothwell]

Hi Paul,

Today's linux-next merge of the audit tree got a conflict in:

  include/uapi/linux/audit.h

between commit:

  7ff89ac608d9 ("audit: add exclude filter extension to feature bitmap")

from Linus' tree and commit:

  0489410368df ("audit: add support for session ID user filter")

from the audit tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc include/uapi/linux/audit.h
index 208df7b44e90,c8dc97bc2c1b..
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@@ -329,11 -330,11 +330,13 @@@ enum 
  #define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT0x0001
  #define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME0x0002
  #define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH  0x0004
 +#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND   0x0008
+ #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER 0x0010
  #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
  AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
  AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
++AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_SESSIONID_FILTER)
  
  /* deprecated: AUDIT_VERSION_* */
  #define AUDIT_VERSION_LATEST  AUDIT_FEATURE_BITMAP_ALL

linux-next: manual merge of the audit tree with Linus' tree

[Stephen Rothwell]

Hi Paul,

Today's linux-next merge of the audit tree got a conflict in:

  include/uapi/linux/audit.h

between commit:

  7ff89ac608d9 ("audit: add exclude filter extension to feature bitmap")

from Linus' tree and commit:

  0489410368df ("audit: add support for session ID user filter")

from the audit tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc include/uapi/linux/audit.h
index 208df7b44e90,c8dc97bc2c1b..
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@@ -329,11 -330,11 +330,13 @@@ enum 
  #define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT0x0001
  #define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME0x0002
  #define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH  0x0004
 +#define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND   0x0008
+ #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER 0x0010
  #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
  AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
  AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
- AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND)
++AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
+ AUDIT_FEATURE_BITMAP_SESSIONID_FILTER)
  
  /* deprecated: AUDIT_VERSION_* */
  #define AUDIT_VERSION_LATEST  AUDIT_FEATURE_BITMAP_ALL

Re: [GIT PULL 4/4] bcm2835-defconfig-64-next-2016-11-18

[Florian Fainelli]

Le 18/11/2016 à 10:58, Eric Anholt a écrit :
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/anholt/linux tags/bcm2835-defconfig-64-next-2016-11-18
> 
> for you to fetch changes up to ac178e4280e65f4d0d14b13a7bfec3a43ff90e66:
> 
>   ARM64: bcm2835: add thermal driver to default config (2016-11-11 09:00:00 
> -0800)
> 
> 
> This pull enables the BCM2837 (Pi 3) thermal driver in the defconfig.
> 
> 

Merged, thanks!
-- 
Florian

Re: [GIT PULL 4/4] bcm2835-defconfig-64-next-2016-11-18

[Florian Fainelli]

Le 18/11/2016 à 10:58, Eric Anholt a écrit :
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/anholt/linux tags/bcm2835-defconfig-64-next-2016-11-18
> 
> for you to fetch changes up to ac178e4280e65f4d0d14b13a7bfec3a43ff90e66:
> 
>   ARM64: bcm2835: add thermal driver to default config (2016-11-11 09:00:00 
> -0800)
> 
> 
> This pull enables the BCM2837 (Pi 3) thermal driver in the defconfig.
> 
> 

Merged, thanks!
-- 
Florian