date:20170516

Re: [PATCH v2 02/10] pinctrl: sh-pfc: r8a7796: add Audio SSI pin support

2017-05-16 Thread Geert Uytterhoeven

Hi Morimoto-san,

On Tue, May 16, 2017 at 3:25 AM, Kuninori Morimoto
 wrote:
>> >> Does this affect H3 ES1.0, too?
>> >> My main worry there is that the name "ssi34_ctrl" is part of the DT ABI,
>> >> so we have to keep that as an alias for "ssi349_ctrl".
>> >
>> > I think this is from Gen3, not ESx.
>> > And no-one is using SSI34 on upstream code.
>> > Should we care about out-of-tree user ?
>>
>> Probably not, especially for H3 ES1.x.
>
> Sorry, I didn't explain.
> This is from Gen3, and ES1.x version datasheet has typo.
> But datasheet team no more update for it.
>
> For example on v0.5 datasheet (= for ES1.x)
>
> 6. Pin Function Controller (PFC)
> 6.2.2  GPIO/Peripheral Function Select Register (GPSR0-7)
>
> This has SSI_xxx_0129 (= this is typo), but
>
> 41. Serial Sound Interface Unit (SSIU)
> 41.1.3 External Pins
>
> This has SSI_xxx_01239

OK.

> Can you agree about SSI_xxx_01239 / SSI_349 are from Gen3, not ES1.x ?

Yes, I agree.

>> > OK. will do in v3
>>
>> OK, thanks!
>
> Oops, sorry, not v3, but incremental patch.
> I will post this patch (= fixup SSI_349) if I could find my posted patches
> on your sh-pfc-for-v4.13.

Oops,I thought I had pushed {clk-renesas,sh-pfc}-for-v4.13 yesterday, but
apparently I got interrupted. Done. But kernel.org mirroring seems to be slow
(latest are based on v4.12-rc1, not v4.11-rc1).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Laurent Pinchart

Hi Sricharan,

On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
> On 2017-05-16 03:04, Laurent Pinchart wrote:
> > On Monday 15 May 2017 23:37:16 Laurent Pinchart wrote:
> >> On Wednesday 03 May 2017 15:54:59 Sricharan R wrote:
> >>> On 5/3/2017 3:24 PM, Robin Murphy wrote:
>  On 02/05/17 19:35, Geert Uytterhoeven wrote:
> > On Fri, Feb 3, 2017 at 4:48 PM, Sricharan R wrote:
> >> From: Laurent Pinchart 
> >> 
> >> Failures to look up an IOMMU when parsing the DT iommus property
> >> need to be handled separately from the .of_xlate() failures to
> >> support deferred probing.
> >> 
> >> The lack of a registered IOMMU can be caused by the lack of a driver
> >> for the IOMMU, the IOMMU device probe not having been performed yet,
> >> having been deferred, or having failed.
> >> 
> >> The first case occurs when the device tree describes the bus master
> >> and IOMMU topology correctly but no device driver exists for the
> >> IOMMU yet or the device driver has not been compiled in. Return NULL,
> >> the caller will configure the device without an IOMMU.
> >> 
> >> The second and third cases are handled by deferring the probe of the
> >> bus master device which will eventually get reprobed after the
> >> IOMMU.
> >> 
> >> The last case is currently handled by deferring the probe of the bus
> >> master device as well. A mechanism to either configure the bus
> >> master device without an IOMMU or to fail the bus master device probe
> >> depending on whether the IOMMU is optional or mandatory would be a
> >> good enhancement.
> >> 
> >> Tested-by: Marek Szyprowski 
> >> Signed-off-by: Laurent Pichart
> >> 
> >> Signed-off-by: Sricharan R 
> > 
> > This patch broke Renesas R-Car Gen3 platforms in renesas-drivers.
> > As the IOMMU nodes in DT are not yet enabled, all devices having
> > iommus properties in DT now fail to probe.
>  
>  How exactly do they fail to probe? Per d7b0558230e4, if there are no
>  ops registered then they should merely defer until we reach the point
>  of giving up and ignoring the IOMMU. Is it just that you have no other
>  late-probing drivers or post-init module loads to kick the deferred
>  queue after that point? I did try to find a way to explicitly kick it
>  from a suitably late initcall, but there didn't seem to be any obvious
>  public interface - anyone have any suggestions?
>  
>  I think that's more of a general problem with the probe deferral
>  mechanism itself (I've seen the same thing happen with some of the
>  CoreSight stuff on Juno due to the number of inter-component
>  dependencies) rather than any specific fault of this series.
> >>> 
> >>> I was thinking of an additional check like below to avoid the
> >>> situation ?
> >>> 
> >>> From 499b6e662f60f23740b8880882b0a16f16434501 Mon Sep 17 00:00:00 2001
> >>> From: Sricharan R 
> >>> Date: Wed, 3 May 2017 13:16:59 +0530
> >>> Subject: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER
> >>> 
> >>> While returning EPROBE_DEFER for iommu masters
> >>> take in to account of iommu nodes that could be
> >>> marked in DT as 'status=disabled', in which case
> >>> simply return NULL and let the master's probe
> >>> continue rather than deferring.
> >>> 
> >>> Signed-off-by: Sricharan R 
> >>> ---
> >>> 
> >>>  drivers/iommu/of_iommu.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>> 
> >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> >>> index 9f44ee8..e6e9bec 100644
> >>> --- a/drivers/iommu/of_iommu.c
> >>> +++ b/drivers/iommu/of_iommu.c
> >>> @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct
> >>> device_node *np)
> >>> 
> >>> ops = iommu_ops_from_fwnode(fwnode);
> >>> if ((ops && !ops->of_xlate) ||
> >>> +   !of_device_is_available(iommu_spec->np) ||
> >>> (!ops && !of_iommu_driver_present(iommu_spec->np)))
> >>> return NULL;
> >> 
> >> This looks good to me, but won't be enough. The ipmmu-vmsa driver in
> >> v4.12-rc1 doesn't call iommu_device_register() and thus won't be found
> >> by iommu_ops_from_fwnode(). Furthermore, it doesn't IOMMU_OF_DECLARE(),
> >> and thus will always be considered as absent.
> >> 
> >> I agree that the ipmmu-vmsa driver needs to be fixed, but it would
> >> have been nice to check existing IOMMU drivers before merging this patch
> >> series...
> > 
> > Please pardon the question, but has this patch series been tested on
> > ARM32 ?
> > 
> > When the device is probed the arch_setup_dma_ops() function is called.
> > It sets the device's dma_ops and the mapping (in
> > __arm_iommu_attach_device()). If probe is deferred,
> > arch_teardown_dma_ops() is called which in turn calls
> > arch_teardown_dma_ops(). This removes the mapping but doesn't touch the
> > dma_ops. The next time the device is probed, arch_s

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Geert Uytterhoeven

Hi Niklas,

On Fri, May 12, 2017 at 11:12 PM, Niklas Söderlund
 wrote:
> On 2017-05-12 16:43:55 +0200, Geert Uytterhoeven wrote:
>> On Fri, May 12, 2017 at 4:32 PM, Niklas Söderlund
>>  wrote:
>> > On 2017-05-12 14:58:53 +0200, Niklas Söderlund wrote:
>> >> On 2017-05-12 14:47:53 +0200, Geert Uytterhoeven wrote:
>> >> > On Fri, May 12, 2017 at 12:27 AM, Niklas Söderlund
>> >> >  wrote:
>> >> > When using PSCI suspend/resume, and Wake-on-LAN cannot work due to
>> >> > PSCI firmware issues, Ethernet fails to come up afterwards:
>> >> >
>> >> > ravb e680.ethernet eth0: failed to switch device to config mode
>> >> > ravb e680.ethernet eth0: device will be stopped after h/w
>> >> > processes are done.
>> >> > ravb e680.ethernet eth0: failed to switch device to config mode
>> >> > dpm_run_callback(): ravb_resume+0x0/0x148 returns -110
>> >> > PM: Device e680.ethernet failed to resume: error -110
>> >> >
>> >> > Your resume routine cannot assume RAVB is in a sane mode, as it will
>> >> > have been reset if PSCI suspend was used.
>> >>
>> >> Ouch, yes this is true thanks for reporting will look in to it.
>> >>
>> >> The problem is that in the resume handler if WoL is enabled it will try
>> >> to close the device before reinitializing it from reset state. If WoL is
>> >> not enabled the device will be closed at suspend time so no need to
>> >> close it before restoring operation from reset in the resume handler.

>> With renesas-drivers, it will refuse to use PSCI suspend if any other wake-up
>> sources are configured, i.e. try
>>
>> echo disabled > /sys/devices/platform/soc/e680.ethernet/power/wakeup
>>
>> Good luck, and have a nice weekend!
>
> Thanks this allowed me to reproduce the same error as you. And after
> future digging I don't believe the problem being in the logic of the
> ravb suspend/resume functions. The problem is that the module clock is
> never turned on after PCSI system suspend if its usage count is above 0
> at suspend time, so the errors we both now observe are due to the module
> clock being disabled.
>
> If I set DEBUG in renesas-cpg-mssr.c and observe when each module clock
> is turned OFF and ON, the fault is clear. If WoL is enabled the clock is
> never turned on when the system is resuming, while if WoL is disabled it
> is. I verified this by removing the calls to clk_enable() and
> clk_disable() from this patch, and by doing so the PCSI system suspend
> works perfect with WoL enabled and the ravb comes up fine after toggling
> SW23 (while ofc WoL no longer works in s2idle due to the module clock is
> switched off at suspend time).
>
> I checked drivers/clk/renesas and I can't find a suspend/resume handler
> for the clock driver, how is this intended to work? If a clock have a
> usage count higher then 0 when the system is PSCI System Suspended it
> seems like it won't be turned back on when the system is resumed from
> this sleep stage. I might have misunderstood something and I need to
> alter the logic in the ravb driver to let the clock driver know it
> should turn on the clock at resume time?

Ah, you found a real use case for suspend/resume support in the clock
drivers ;-)

Due to PSCI system suspend powering down the whole SoC, all clock
settings are lost.

Thanks, I will look into this...

> Whit all this being said I still like to withdraw this patch as I found
> another fault with it, ravb_wol_restore() will unconditionally be called
> while ravb_wol_setup() will only be called if netif_running(ndev). This
> is en easy fix and I will send out a v2 once we figure out what to do
> about the clock.

The clock issue is external to the ravb driver. If it works with
s2idle, it should
be OK.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH] pinctrl: sh-pfc: r8a7795: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto

From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}_349 instead of SSI_{WS/SCK}_34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 0454f31..c31881b 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -193,8 +193,8 @@
 #define GPSR6_9F_(SSI_WS4, IP15_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP15_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP15_19_16)
-#define GPSR6_6F_(SSI_WS34,IP15_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP15_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP15_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP15_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP15_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP15_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP14_31_28)
@@ -339,8 +339,8 @@
 #define IP14_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1315,11 +1315,11 @@ enum {
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP15_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP15_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP15_15_12,HCTS2_N_A,  SEL_HSCIF2_0),
PINMUX_IPSR_MSEL(IP15_15_12,MSIOF1_SS2_A,   SEL_MSIOF1_0),

[PATCH] pinctrl: sh-pfc: r8a7795-es1: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto

From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}_349 instead of SSI_{WS/SCK}_34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
index 081efda..e6a967a 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
@@ -192,8 +192,8 @@
 #define GPSR6_9F_(SSI_WS4, IP14_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP14_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP14_19_16)
-#define GPSR6_6F_(SSI_WS34,IP14_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP14_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP14_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP14_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP14_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP14_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP13_31_28)
@@ -328,8 +328,8 @@
 #define IP13_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP14_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP14_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP14_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP14_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP14_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP14_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1256,11 +1256,11 @@ enum {
PINMUX_IPSR_MSEL(IP14_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP14_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP14_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP14_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP14_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP14_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP14_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP14_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP14_15_12,HCTS2_N_A,  SEL_HSCIF2_0),
PINMUX_IPSR_MSEL(IP14_15_12,MSIOF1_SS2_A

[PATCH] pinctrl: sh-pfc: r8a7796: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto


From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}_349 instead of SSI_{WS/SCK}_34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
 drivers/pinctrl/sh-pfc/pfc-r8a7796.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
index 51f9e6e..5ae752f 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
@@ -199,8 +199,8 @@
 #define GPSR6_9F_(SSI_WS4, IP15_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP15_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP15_19_16)
-#define GPSR6_6F_(SSI_WS34,IP15_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP15_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP15_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP15_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP15_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP15_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP14_31_28)
@@ -345,8 +345,8 @@
 #define IP14_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1319,11 +1319,11 @@ enum {
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP15_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP15_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP15_15_12,HCTS2_N_A,  SEL_HSCIF2_0),
PINMUX_IPSR_MSEL(IP15_15_12,MSIOF1_SS2_A,   SEL_MSI

Re: [PATCH] pinctrl: sh-pfc: r8a7795-es1: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Geert Uytterhoeven

Hi Morimoto-san,

On Tue, May 16, 2017 at 10:01 AM, Kuninori Morimoto
 wrote:
> From: Kuninori Morimoto 
>
> R-Car Gen3 is using SSI_{WS/SCK}_349 instead of SSI_{WS/SCK}_34.
> But, current code is based on old datasheet which had typo.
> This patch fixes this typo.
>
> Signed-off-by: Kuninori Morimoto 

You forgot to change the string in ssi_groups[], like below?

@@ -4509,7 +4509,7 @@ static const char * const ssi_groups[] = {
"ssi2_ctrl_a",
"ssi2_ctrl_b",
"ssi3_data",
-   "ssi34_ctrl",
+   "ssi349_ctrl",
"ssi4_data",
"ssi4_ctrl",
"ssi5_data",

Without that change, it won't work.

If you care about backwards compatibility (I thought you didn't?),
you have to keep the string "ssi34_ctrl", and add "ssi349_ctrl",
and add an open-coded entry in pinmux_group[]  like:

{
.name = "ssi34_ctrl",
.pins = ssi349_pins,
.mux = ssi349_mux,
.nr_pins = ARRAY_SIZE(ssi349_pins),
}

Or add a new macro SH_PFC_PIN_GROUP_ALIAS(n, alias).

The same comment applies to pfc-r8a7796.c.

If the intention was to change the string, I can make that change myself,
no need to resent.

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] pinctrl: sh-pfc: r8a7795: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Geert Uytterhoeven

On Tue, May 16, 2017 at 10:01 AM, Kuninori Morimoto
 wrote:
> From: Kuninori Morimoto 
>
> R-Car Gen3 is using SSI_{WS/SCK}_349 instead of SSI_{WS/SCK}_34.
> But, current code is based on old datasheet which had typo.
> This patch fixes this typo.
>
> Signed-off-by: Kuninori Morimoto 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] pinctrl: sh-pfc: r8a7795-es1: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto


Hi Geert

> You forgot to change the string in ssi_groups[], like below?
> 
> @@ -4509,7 +4509,7 @@ static const char * const ssi_groups[] = {
> "ssi2_ctrl_a",
> "ssi2_ctrl_b",
> "ssi3_data",
> -   "ssi34_ctrl",
> +   "ssi349_ctrl",
> "ssi4_data",
> "ssi4_ctrl",
> "ssi5_data",

Grr, indeed.
I will send v2

> If you care about backwards compatibility (I thought you didn't?),
(snip)
> If the intention was to change the string, I can make that change myself,
> no need to resent.

ssi34 is not used in upstream, and I don't care about out-of-tree code.

Best regards
---
Kuninori Morimoto

[PATCH v2] pinctrl: sh-pfc: r8a7795-es1: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto

From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
v1 -> v2

 - tidyup git log (_349 -> 349)
 - "ssi34_ctrl" -> "ssi349_ctrl"

 drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
index 081efda..95fd099 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795-es1.c
@@ -192,8 +192,8 @@
 #define GPSR6_9F_(SSI_WS4, IP14_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP14_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP14_19_16)
-#define GPSR6_6F_(SSI_WS34,IP14_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP14_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP14_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP14_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP14_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP14_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP13_31_28)
@@ -328,8 +328,8 @@
 #define IP13_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP14_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP14_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP14_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP14_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP14_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP14_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP14_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1256,11 +1256,11 @@ enum {
PINMUX_IPSR_MSEL(IP14_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP14_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP14_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP14_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP14_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP14_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP14_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP14_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP14_15_12,HCTS2_N_A,

[PATCH v2] pinctrl: sh-pfc: r8a7795: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto

From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
v1 -> v2

 - tidyup git log (_349 -> 349)

 drivers/pinctrl/sh-pfc/pfc-r8a7795.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
index 0454f31..c31881b 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7795.c
@@ -193,8 +193,8 @@
 #define GPSR6_9F_(SSI_WS4, IP15_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP15_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP15_19_16)
-#define GPSR6_6F_(SSI_WS34,IP15_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP15_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP15_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP15_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP15_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP15_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP14_31_28)
@@ -339,8 +339,8 @@
 #define IP14_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1315,11 +1315,11 @@ enum {
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP15_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP15_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP15_15_12,HCTS2_N_A,  SEL_HSCIF2_0),
PINMUX_IPSR_MSEL(IP15_15_12,

[PATCH v2] pinctrl: sh-pfc: r8a7796: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto

From: Kuninori Morimoto 

R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
But, current code is based on old datasheet which had typo.
This patch fixes this typo.

Signed-off-by: Kuninori Morimoto 
---
v1 -> v2

 - tidyup git log (_349 -> 349)
 - "ssi34_ctrl" -> "ssi349_ctrl"

 drivers/pinctrl/sh-pfc/pfc-r8a7796.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
index 51f9e6e..98bf5d0 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7796.c
@@ -199,8 +199,8 @@
 #define GPSR6_9F_(SSI_WS4, IP15_27_24)
 #define GPSR6_8F_(SSI_SCK4,IP15_23_20)
 #define GPSR6_7F_(SSI_SDATA3,  IP15_19_16)
-#define GPSR6_6F_(SSI_WS34,IP15_15_12)
-#define GPSR6_5F_(SSI_SCK34,   IP15_11_8)
+#define GPSR6_6F_(SSI_WS349,   IP15_15_12)
+#define GPSR6_5F_(SSI_SCK349,  IP15_11_8)
 #define GPSR6_4F_(SSI_SDATA2_A,IP15_7_4)
 #define GPSR6_3F_(SSI_SDATA1_A,IP15_3_0)
 #define GPSR6_2F_(SSI_SDATA0,  IP14_31_28)
@@ -345,8 +345,8 @@
 #define IP14_31_28 FM(SSI_SDATA0)  F_(0, 0)
FM(MSIOF1_SS2_F)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_3_0   FM(SSI_SDATA1_A)F_(0, 0)F_(0, 0)
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_7_4   FM(SSI_SDATA2_A)F_(0, 0)F_(0, 0)
F_(0, 0)FM(SSI_SCK1_B)  F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_11_8  FM(SSI_SCK34)   F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
-#define IP15_15_12 FM(SSI_WS34)FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_11_8  FM(SSI_SCK349)  F_(0, 0)
FM(MSIOF1_SS1_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_OPWM_0_A)F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
+#define IP15_15_12 FM(SSI_WS349)   FM(HCTS2_N_A)   
FM(MSIOF1_SS2_A)F_(0, 0)F_(0, 0)F_(0, 
0)FM(STP_IVCXO27_0_A) F_(0, 0)F_(0, 0)F_(0, 
0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) F_(0, 0)
 #define IP15_19_16 FM(SSI_SDATA3)  FM(HRTS2_N_A)   
FM(MSIOF1_TXD_A)F_(0, 0)F_(0, 0)
FM(TS_SCK0_A)   FM(STP_ISCLK_0_A)   FM(RIF0_D1_A)   FM(RIF2_D0_A)   
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_23_20 FM(SSI_SCK4)FM(HRX2_A)  
FM(MSIOF1_SCK_A)F_(0, 0)F_(0, 0)
FM(TS_SDAT0_A)  FM(STP_ISD_0_A) FM(RIF0_CLK_A)  FM(RIF2_CLK_A)  
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
 #define IP15_27_24 FM(SSI_WS4) FM(HTX2_A)  
FM(MSIOF1_SYNC_A)   F_(0, 0)F_(0, 0)
FM(TS_SDEN0_A)  FM(STP_ISEN_0_A)FM(RIF0_SYNC_A) FM(RIF2_SYNC_A) 
F_(0, 0)F_(0, 0)F_(0, 0)F_(0, 0) F_(0, 0) F_(0, 0) 
F_(0, 0)
@@ -1319,11 +1319,11 @@ enum {
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SDATA2_A,   SEL_SSI_0),
PINMUX_IPSR_MSEL(IP15_7_4,  SSI_SCK1_B, SEL_SSI_1),
 
-   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK34),
+   PINMUX_IPSR_GPSR(IP15_11_8, SSI_SCK349),
PINMUX_IPSR_MSEL(IP15_11_8, MSIOF1_SS1_A,   SEL_MSIOF1_0),
PINMUX_IPSR_MSEL(IP15_11_8, STP_OPWM_0_A,   SEL_SSP1_0_0),
 
-   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS34),
+   PINMUX_IPSR_GPSR(IP15_15_12,SSI_WS349),
PINMUX_IPSR_MSEL(IP15_15_12,HCTS2_N_A,  SEL_HSCIF2_

Re: [PATCH v2] pinctrl: sh-pfc: r8a7795-es1: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Geert Uytterhoeven

On Tue, May 16, 2017 at 10:42 AM, Kuninori Morimoto
 wrote:
> From: Kuninori Morimoto 
>
> R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
> But, current code is based on old datasheet which had typo.
> This patch fixes this typo.
>
> Signed-off-by: Kuninori Morimoto 
> ---
> v1 -> v2

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2] pinctrl: sh-pfc: r8a7796: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Geert Uytterhoeven

On Tue, May 16, 2017 at 10:42 AM, Kuninori Morimoto
 wrote:
> From: Kuninori Morimoto 
>
> R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
> But, current code is based on old datasheet which had typo.
> This patch fixes this typo.
>
> Signed-off-by: Kuninori Morimoto 
> ---
> v1 -> v2

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

>  - tidyup git log (_349 -> 349)

BTW, I fixed up your r8a7795 patch with the same change while applying.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Niklas Söderlund

Hi Geert,

Thanks for your feedback.

On 2017-05-16 09:48:51 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Fri, May 12, 2017 at 11:12 PM, Niklas Söderlund
>  wrote:
> > On 2017-05-12 16:43:55 +0200, Geert Uytterhoeven wrote:
> >> On Fri, May 12, 2017 at 4:32 PM, Niklas Söderlund
> >>  wrote:
> >> > On 2017-05-12 14:58:53 +0200, Niklas Söderlund wrote:
> >> >> On 2017-05-12 14:47:53 +0200, Geert Uytterhoeven wrote:
> >> >> > On Fri, May 12, 2017 at 12:27 AM, Niklas Söderlund
> >> >> >  wrote:
> >> >> > When using PSCI suspend/resume, and Wake-on-LAN cannot work due to
> >> >> > PSCI firmware issues, Ethernet fails to come up afterwards:
> >> >> >
> >> >> > ravb e680.ethernet eth0: failed to switch device to config 
> >> >> > mode
> >> >> > ravb e680.ethernet eth0: device will be stopped after h/w
> >> >> > processes are done.
> >> >> > ravb e680.ethernet eth0: failed to switch device to config 
> >> >> > mode
> >> >> > dpm_run_callback(): ravb_resume+0x0/0x148 returns -110
> >> >> > PM: Device e680.ethernet failed to resume: error -110
> >> >> >
> >> >> > Your resume routine cannot assume RAVB is in a sane mode, as it will
> >> >> > have been reset if PSCI suspend was used.
> >> >>
> >> >> Ouch, yes this is true thanks for reporting will look in to it.
> >> >>
> >> >> The problem is that in the resume handler if WoL is enabled it will try
> >> >> to close the device before reinitializing it from reset state. If WoL is
> >> >> not enabled the device will be closed at suspend time so no need to
> >> >> close it before restoring operation from reset in the resume handler.
> 
> >> With renesas-drivers, it will refuse to use PSCI suspend if any other 
> >> wake-up
> >> sources are configured, i.e. try
> >>
> >> echo disabled > 
> >> /sys/devices/platform/soc/e680.ethernet/power/wakeup
> >>
> >> Good luck, and have a nice weekend!
> >
> > Thanks this allowed me to reproduce the same error as you. And after
> > future digging I don't believe the problem being in the logic of the
> > ravb suspend/resume functions. The problem is that the module clock is
> > never turned on after PCSI system suspend if its usage count is above 0
> > at suspend time, so the errors we both now observe are due to the module
> > clock being disabled.
> >
> > If I set DEBUG in renesas-cpg-mssr.c and observe when each module clock
> > is turned OFF and ON, the fault is clear. If WoL is enabled the clock is
> > never turned on when the system is resuming, while if WoL is disabled it
> > is. I verified this by removing the calls to clk_enable() and
> > clk_disable() from this patch, and by doing so the PCSI system suspend
> > works perfect with WoL enabled and the ravb comes up fine after toggling
> > SW23 (while ofc WoL no longer works in s2idle due to the module clock is
> > switched off at suspend time).
> >
> > I checked drivers/clk/renesas and I can't find a suspend/resume handler
> > for the clock driver, how is this intended to work? If a clock have a
> > usage count higher then 0 when the system is PSCI System Suspended it
> > seems like it won't be turned back on when the system is resumed from
> > this sleep stage. I might have misunderstood something and I need to
> > alter the logic in the ravb driver to let the clock driver know it
> > should turn on the clock at resume time?
> 
> Ah, you found a real use case for suspend/resume support in the clock
> drivers ;-)

Happy to be of service ;-)

> 
> Due to PSCI system suspend powering down the whole SoC, all clock
> settings are lost.
> 
> Thanks, I will look into this...

Thanks!

> 
> > Whit all this being said I still like to withdraw this patch as I found
> > another fault with it, ravb_wol_restore() will unconditionally be called
> > while ravb_wol_setup() will only be called if netif_running(ndev). This
> > is en easy fix and I will send out a v2 once we figure out what to do
> > about the clock.
> 
> The clock issue is external to the ravb driver. If it works with
> s2idle, it should
> be OK.

Do you think it's a good idea to move ahead with a v2 of the ravb WoL 
patch to fix the unrelated issue and aim for it to be picked up prior to
suspend/resume support is added to the CPG/MSSR?

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Geert Uytterhoeven

Hi Niklas,

On Tue, May 16, 2017 at 11:02 AM, Niklas Söderlund
 wrote:
>> > Whit all this being said I still like to withdraw this patch as I found
>> > another fault with it, ravb_wol_restore() will unconditionally be called
>> > while ravb_wol_setup() will only be called if netif_running(ndev). This
>> > is en easy fix and I will send out a v2 once we figure out what to do
>> > about the clock.
>>
>> The clock issue is external to the ravb driver. If it works with
>> s2idle, it should
>> be OK.
>
> Do you think it's a good idea to move ahead with a v2 of the ravb WoL
> patch to fix the unrelated issue and aim for it to be picked up prior to
> suspend/resume support is added to the CPG/MSSR?

Sure.

It can still be used on R-Car Gen2, where we're not s*d by mandatory PSCI.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Sakari Ailus

Hi Laurent,

On Tue, May 16, 2017 at 10:17:08AM +0300, Laurent Pinchart wrote:
> Hi Sricharan,
> 
> On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
> > On 2017-05-16 03:04, Laurent Pinchart wrote:
> > > On Monday 15 May 2017 23:37:16 Laurent Pinchart wrote:
> > >> On Wednesday 03 May 2017 15:54:59 Sricharan R wrote:
> > >>> On 5/3/2017 3:24 PM, Robin Murphy wrote:
> >  On 02/05/17 19:35, Geert Uytterhoeven wrote:
> > > On Fri, Feb 3, 2017 at 4:48 PM, Sricharan R wrote:
> > >> From: Laurent Pinchart 
> > >> 
> > >> Failures to look up an IOMMU when parsing the DT iommus property
> > >> need to be handled separately from the .of_xlate() failures to
> > >> support deferred probing.
> > >> 
> > >> The lack of a registered IOMMU can be caused by the lack of a driver
> > >> for the IOMMU, the IOMMU device probe not having been performed yet,
> > >> having been deferred, or having failed.
> > >> 
> > >> The first case occurs when the device tree describes the bus master
> > >> and IOMMU topology correctly but no device driver exists for the
> > >> IOMMU yet or the device driver has not been compiled in. Return NULL,
> > >> the caller will configure the device without an IOMMU.
> > >> 
> > >> The second and third cases are handled by deferring the probe of the
> > >> bus master device which will eventually get reprobed after the
> > >> IOMMU.
> > >> 
> > >> The last case is currently handled by deferring the probe of the bus
> > >> master device as well. A mechanism to either configure the bus
> > >> master device without an IOMMU or to fail the bus master device probe
> > >> depending on whether the IOMMU is optional or mandatory would be a
> > >> good enhancement.
> > >> 
> > >> Tested-by: Marek Szyprowski 
> > >> Signed-off-by: Laurent Pichart
> > >> 
> > >> Signed-off-by: Sricharan R 
> > > 
> > > This patch broke Renesas R-Car Gen3 platforms in renesas-drivers.
> > > As the IOMMU nodes in DT are not yet enabled, all devices having
> > > iommus properties in DT now fail to probe.
> >  
> >  How exactly do they fail to probe? Per d7b0558230e4, if there are no
> >  ops registered then they should merely defer until we reach the point
> >  of giving up and ignoring the IOMMU. Is it just that you have no other
> >  late-probing drivers or post-init module loads to kick the deferred
> >  queue after that point? I did try to find a way to explicitly kick it
> >  from a suitably late initcall, but there didn't seem to be any obvious
> >  public interface - anyone have any suggestions?
> >  
> >  I think that's more of a general problem with the probe deferral
> >  mechanism itself (I've seen the same thing happen with some of the
> >  CoreSight stuff on Juno due to the number of inter-component
> >  dependencies) rather than any specific fault of this series.
> > >>> 
> > >>> I was thinking of an additional check like below to avoid the
> > >>> situation ?
> > >>> 
> > >>> From 499b6e662f60f23740b8880882b0a16f16434501 Mon Sep 17 00:00:00 2001
> > >>> From: Sricharan R 
> > >>> Date: Wed, 3 May 2017 13:16:59 +0530
> > >>> Subject: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER
> > >>> 
> > >>> While returning EPROBE_DEFER for iommu masters
> > >>> take in to account of iommu nodes that could be
> > >>> marked in DT as 'status=disabled', in which case
> > >>> simply return NULL and let the master's probe
> > >>> continue rather than deferring.
> > >>> 
> > >>> Signed-off-by: Sricharan R 
> > >>> ---
> > >>> 
> > >>>  drivers/iommu/of_iommu.c | 1 +
> > >>>  1 file changed, 1 insertion(+)
> > >>> 
> > >>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > >>> index 9f44ee8..e6e9bec 100644
> > >>> --- a/drivers/iommu/of_iommu.c
> > >>> +++ b/drivers/iommu/of_iommu.c
> > >>> @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct
> > >>> device_node *np)
> > >>> 
> > >>> ops = iommu_ops_from_fwnode(fwnode);
> > >>> if ((ops && !ops->of_xlate) ||
> > >>> +   !of_device_is_available(iommu_spec->np) ||
> > >>> (!ops && !of_iommu_driver_present(iommu_spec->np)))
> > >>> return NULL;
> > >> 
> > >> This looks good to me, but won't be enough. The ipmmu-vmsa driver in
> > >> v4.12-rc1 doesn't call iommu_device_register() and thus won't be found
> > >> by iommu_ops_from_fwnode(). Furthermore, it doesn't IOMMU_OF_DECLARE(),
> > >> and thus will always be considered as absent.
> > >> 
> > >> I agree that the ipmmu-vmsa driver needs to be fixed, but it would
> > >> have been nice to check existing IOMMU drivers before merging this patch
> > >> series...
> > > 
> > > Please pardon the question, but has this patch series been tested on
> > > ARM32 ?
> > > 
> > > When the device is probed the arch_setup_dma_ops() function is called.
> > > It sets t

Re: [PATCH v2] pinctrl: sh-pfc: r8a7796: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Geert Uytterhoeven

Hi Morimoto-san,

On Tue, May 16, 2017 at 10:50 AM, Geert Uytterhoeven
 wrote:
> On Tue, May 16, 2017 at 10:42 AM, Kuninori Morimoto
>  wrote:
>> From: Kuninori Morimoto 
>>
>> R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
>> But, current code is based on old datasheet which had typo.
>> This patch fixes this typo.
>>
>> Signed-off-by: Kuninori Morimoto 
>> ---
>> v1 -> v2
>
> Reviewed-by: Geert Uytterhoeven 
> i.e. will queue in sh-pfc-for-v4.13.

As I haven't send a pull request yet, I'm moving the definition part of the
rename up, and folded the ssi349_ctrl part into "pinctrl: sh-pfc: r8a7796: Add
Audio clock pin support".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v5 3/3] phy: Group vendor specific phy drivers

2017-05-16 Thread Kishon Vijay Abraham I

Hi Vivek,

On Thursday 11 May 2017 12:17 PM, Vivek Gautam wrote:
> Adding vendor specific directories in phy to group
> phy drivers under their respective vendor umbrella.
> 
> Also updated the MAINTAINERS file to reflect the correct
> directory structure for phy drivers.
> 
> Signed-off-by: Vivek Gautam 
> Acked-by: Heiko Stuebner 
> Acked-by: Viresh Kumar 
> Acked-by: Krzysztof Kozlowski 
> Acked-by: Yoshihiro Shimoda 
> Reviewed-by: Jaehoon Chung 
> Cc: Kishon Vijay Abraham I 
> Cc: David S. Miller 
> Cc: Geert Uytterhoeven 
> Cc: Yoshihiro Shimoda 
> Cc: Guenter Roeck 
> Cc: Heiko Stuebner 
> Cc: Viresh Kumar 
> Cc: Maxime Ripard 
> Cc: Chen-Yu Tsai 
> Cc: Sylwester Nawrocki 
> Cc: Krzysztof Kozlowski 
> Cc: Jaehoon Chung 
> Cc: Stephen Boyd 
> Cc: Martin Blumenstingl 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-o...@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> ---
> 
> Changes since v4:
>  - Reprepared the patch based on latest torvalds/master.
>  - Added new directory for amlogic, since there's a new patch [1]
>coming in for amlogic platforms.
> 
> Changes since v3:
>  - Added 'Acked-by' and 'Reviewed by' tags received for this patch.
>  - No functional change.
> 
> Changes since v2:
>  - Rebased on linux-phy/next.
>  - Took care of drivers added/removed for each vendors since v2.
>  - Updated the 'Signed-off-by' tag with my current email id.
> 
> Changes from v1:
>  - Updated the MAINTAINERS file to reflect the same change
>in directory structure.
>  - Removed PHY_PLAT config as pointed out by Kishon.
>So we don't require the second patch in the v1 of this series:
>[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency
>  - Renamed sunxi --> allwinner; rcar --> renesas.
>  - Fixed error coming with qcom Makefile.
> 
> * Build tested multi_v7_defconfig.
> * Build tested arm64 defconfig with all the involved configs enabled.
> 
> [1] https://patchwork.kernel.org/patch/9684303/
>
.
.

.
.
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index afaf7b643eeb..b353ac603ea0 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,73 +15,6 @@ config GENERIC_PHY
> phy users can obtain reference to the PHY. All the users of this

.
.

.
.
> -
> +menu "Platform Phy drivers"

I don't think creating a new menu is required. I removed it and merged into
linux-phy.

Thanks
Kishon

Re: [PATCH] pinctrl: sh-pfc: r8a7794: rename some I2C signals

2017-05-16 Thread Geert Uytterhoeven

On Thu, Apr 27, 2017 at 11:17 PM, Sergei Shtylyov
 wrote:
> The R8A7794 PFC driver was apparently based  on the preliminary revisions
> of  the  user's manual which called I2C5 device IIC0 and IIC0 device IIC1.
> Luckily, these signals haven't been used for any functions/groups so far,
> so  the  renaming should  be painless...
>
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 1/2] pinctrl: sh-pfc: r8a7794: remove AVB_AVTP_* groups

2017-05-16 Thread Geert Uytterhoeven

On Fri, Apr 28, 2017 at 7:50 PM, Sergei Shtylyov
 wrote:
> The ATA_AVTP_* signals are documented as reserved in the recent R-Car E2
> user's manual (the only remaining mention is in the table 5.2 and I believe
> it's a simple overlook).  Remove the AVB_AVTP_* pinmux groups -- we will
> remove the signals themselves in the next patch, along with the other now
> reserved bits...
>
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH 2/2] pinctrl: sh-pfc: r8a7794: remove reserved bits

2017-05-16 Thread Geert Uytterhoeven

On Fri, Apr 28, 2017 at 7:50 PM, Sergei Shtylyov
 wrote:
> The R8A7794 PFC driver was apparently based on the preliminary revisions
> of  the  user's manual which  had some signals and MOD_SEL register fields
> described which the recent manual changed  to reserved. Of course, these
> signals haven't ever  been really used, which makes removing them painless.
>
> While at it, make the large *enum* look better by starting a new line each
> time  a new  row  in the IPSR and MOD_SEL register field tables is started.
>
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Geert Uytterhoeven 
i.e. will queue in sh-pfc-for-v4.13.

> PINMUX_IPSR_GPSR(IP2_31_30, A20),
> PINMUX_IPSR_GPSR(IP2_31_30, SPCLK),
> -   PINMUX_IPSR_GPSR(IP2_29_27, MOUT1),

This patch seems to remove quite some wrong numbered entries, good ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Simon Horman

On Tue, May 16, 2017 at 11:07:34AM +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Tue, May 16, 2017 at 11:02 AM, Niklas Söderlund
>  wrote:
> >> > Whit all this being said I still like to withdraw this patch as I found
> >> > another fault with it, ravb_wol_restore() will unconditionally be called
> >> > while ravb_wol_setup() will only be called if netif_running(ndev). This
> >> > is en easy fix and I will send out a v2 once we figure out what to do
> >> > about the clock.
> >>
> >> The clock issue is external to the ravb driver. If it works with
> >> s2idle, it should
> >> be OK.
> >
> > Do you think it's a good idea to move ahead with a v2 of the ravb WoL
> > patch to fix the unrelated issue and aim for it to be picked up prior to
> > suspend/resume support is added to the CPG/MSSR?
> 
> Sure.
> 
> It can still be used on R-Car Gen2, where we're not s*d by mandatory PSCI.

Is there some way for - e.g. the driver - to not enable WoL on Gen3 SoCs
until the clock issues is sorted out? I'm quite happy to enable features
where they work; not so much where they don't.

[PATCH 1/2] i2c: stub: move module_init/exit annotations to the proper place

2017-05-16 Thread Wolfram Sang

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-stub.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-stub.c b/drivers/i2c/i2c-stub.c
index 06af583d510150..0aa4d646f8fb26 100644
--- a/drivers/i2c/i2c-stub.c
+++ b/drivers/i2c/i2c-stub.c
@@ -406,16 +406,15 @@ static int __init i2c_stub_init(void)
i2c_stub_free();
return ret;
 }
+module_init(i2c_stub_init);
 
 static void __exit i2c_stub_exit(void)
 {
i2c_del_adapter(&stub_adapter);
i2c_stub_free();
 }
+module_exit(i2c_stub_exit);
 
 MODULE_AUTHOR("Mark M. Hoffman ");
 MODULE_DESCRIPTION("I2C stub driver");
 MODULE_LICENSE("GPL");
-
-module_init(i2c_stub_init);
-module_exit(i2c_stub_exit);
-- 
2.11.0

[PATCH 2/2] i2c: stub: use pr_fmt

2017-05-16 Thread Wolfram Sang

Instead of hard coding "i2c-stub:", let's use the pr_fmt mechanism to
achieve the same more easily. This makes it easier to stay consistent
when adding new messages. Also, remove an unneeded OOM message while we
are here.

Signed-off-by: Wolfram Sang 
---
 drivers/i2c/i2c-stub.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/i2c-stub.c b/drivers/i2c/i2c-stub.c
index 0aa4d646f8fb26..5627b1df391c7e 100644
--- a/drivers/i2c/i2c-stub.c
+++ b/drivers/i2c/i2c-stub.c
@@ -15,7 +15,8 @@
 GNU General Public License for more details.
 */
 
-#define DEBUG 1
+#define DEBUG
+#define pr_fmt(fmt) "i2c-stub: " fmt
 
 #include 
 #include 
@@ -342,7 +343,7 @@ static int __init i2c_stub_allocate_banks(int i)
if (!chip->bank_words)
return -ENOMEM;
 
-   pr_debug("i2c-stub: Allocated %u banks of %u words each (registers 
0x%02x to 0x%02x)\n",
+   pr_debug("Allocated %u banks of %u words each (registers 0x%02x to 
0x%02x)\n",
 chip->bank_mask, chip->bank_size, chip->bank_start,
 chip->bank_end);
 
@@ -363,28 +364,27 @@ static int __init i2c_stub_init(void)
int i, ret;
 
if (!chip_addr[0]) {
-   pr_err("i2c-stub: Please specify a chip address\n");
+   pr_err("Please specify a chip address\n");
return -ENODEV;
}
 
for (i = 0; i < MAX_CHIPS && chip_addr[i]; i++) {
if (chip_addr[i] < 0x03 || chip_addr[i] > 0x77) {
-   pr_err("i2c-stub: Invalid chip address 0x%02x\n",
+   pr_err("Invalid chip address 0x%02x\n",
   chip_addr[i]);
return -EINVAL;
}
 
-   pr_info("i2c-stub: Virtual chip at 0x%02x\n", chip_addr[i]);
+   pr_info("Virtual chip at 0x%02x\n", chip_addr[i]);
}
 
/* Allocate memory for all chips at once */
stub_chips_nr = i;
stub_chips = kcalloc(stub_chips_nr, sizeof(struct stub_chip),
 GFP_KERNEL);
-   if (!stub_chips) {
-   pr_err("i2c-stub: Out of memory\n");
+   if (!stub_chips)
return -ENOMEM;
-   }
+
for (i = 0; i < stub_chips_nr; i++) {
INIT_LIST_HEAD(&stub_chips[i].smbus_blocks);
 
-- 
2.11.0

[RFC] i2c-stub: make it usable with DT when booting

2017-05-16 Thread Wolfram Sang

This patch makes the stub driver parse the device tree when booting and
create a virtual bus with the desired devices attached. Here is an
example DTS snipplet making use of it. It is for simulating a more
complex camera device which has dependencies which needs to be sorted
out at boot time:

i2c@42 {
compatible = "i2c-stub";

#address-cells = <1>;
#size-cells = <0>;

des@4c {
compatible = "maxim,max9268";
reg = <0x4c>;
};

eeprom@57 {
compatible = "atmel,24c02";
reg = <0x57>;
};
};

FIXME: can fw_* calls be used instead of of_*?

Signed-off-by: Wolfram Sang 
---

It was really useful when developing. When using fw_* calls, we probably could
make use of it with ACPI as well. Still, calling for opinions if we want this
upstream or if we want to stay module-only? Also, the binding should really
be marked as "development only". Or is it better to keep it as a separate
patch to keep the binding un-official?

 drivers/i2c/Kconfig|  2 +-
 drivers/i2c/i2c-stub.c | 34 ++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index efc3354d60ae89..6d38b38632f44c 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -99,7 +99,7 @@ source drivers/i2c/busses/Kconfig
 
 config I2C_STUB
tristate "I2C/SMBus Test Stub"
-   depends on m
+   depends on m || OF
default 'n'
help
  This module may be useful to developers of SMBus client drivers,
diff --git a/drivers/i2c/i2c-stub.c b/drivers/i2c/i2c-stub.c
index 5627b1df391c7e..ce30bc4ac15a28 100644
--- a/drivers/i2c/i2c-stub.c
+++ b/drivers/i2c/i2c-stub.c
@@ -359,10 +359,44 @@ static void i2c_stub_free(void)
kfree(stub_chips);
 }
 
+#ifdef CONFIG_OF
+void i2c_stub_parse_of(void)
+{
+   struct device_node *np, *child;
+   u32 reg, i = 0;
+
+   np = of_find_compatible_node(NULL, NULL, "i2c-stub");
+   if (!np)
+   return;
+
+   for_each_available_child_of_node(np, child) {
+   if (of_property_read_u32(child, "reg", ®) == 0) {
+   chip_addr[i++] = reg;
+   if (i == MAX_CHIPS) {
+   pr_warn("Maximum number of chips reached!\n");
+   break;
+   }
+   }
+   }
+
+   stub_adapter.dev.of_node = np;
+
+   np = of_find_compatible_node(np, NULL, "i2c-stub");
+   if (np) {
+   pr_warn("Driver can currently do only one stub from DT!\n");
+   of_node_put(np);
+   }
+}
+#else
+void i2c_stub_parse_of(void) { }
+#endif
+
 static int __init i2c_stub_init(void)
 {
int i, ret;
 
+   i2c_stub_parse_of();
+
if (!chip_addr[0]) {
pr_err("Please specify a chip address\n");
return -ENODEV;
-- 
2.11.0

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Geert Uytterhoeven

Hi Simon,

On Tue, May 16, 2017 at 1:01 PM, Simon Horman  wrote:
> On Tue, May 16, 2017 at 11:07:34AM +0200, Geert Uytterhoeven wrote:
>> On Tue, May 16, 2017 at 11:02 AM, Niklas Söderlund
>>  wrote:
>> >> > Whit all this being said I still like to withdraw this patch as I found
>> >> > another fault with it, ravb_wol_restore() will unconditionally be called
>> >> > while ravb_wol_setup() will only be called if netif_running(ndev). This
>> >> > is en easy fix and I will send out a v2 once we figure out what to do
>> >> > about the clock.
>> >>
>> >> The clock issue is external to the ravb driver. If it works with
>> >> s2idle, it should
>> >> be OK.
>> >
>> > Do you think it's a good idea to move ahead with a v2 of the ravb WoL
>> > patch to fix the unrelated issue and aim for it to be picked up prior to
>> > suspend/resume support is added to the CPG/MSSR?
>>
>> Sure.
>>
>> It can still be used on R-Car Gen2, where we're not s*d by mandatory PSCI.
>
> Is there some way for - e.g. the driver - to not enable WoL on Gen3 SoCs
> until the clock issues is sorted out? I'm quite happy to enable features

"priv->chip_id != RCAR_GEN3". However, as we don't have RAVB enabled
on any R-Car Gen2 board, its use is limited.

> where they work; not so much where they don't.

Agreed.

One workaround could be to disable/enable the module clock in the WoL
resume path, to make sure it is enabled.  Once the enable count reaches
0, CCF will know it's disabled, and will really enable next time.
You may need a double disable/double enable though, without testing I
don't know remember the enable count is 1 or 2 at that point (due to PM
runtime).

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v4] pinctrl: sh-pfc: r8a7791: add R8A7743 support

2017-05-16 Thread Geert Uytterhoeven

Hi Sergei,

On Thu, Apr 20, 2017 at 8:46 PM, Sergei Shtylyov
 wrote:
> Renesas RZ/G1M (R8A7743) is pin compatible with R-Car M2-W/N (R8A7791/3),
> however it doesn't have several automotive specific peripherals. Annotate
> all the items that only exist on the R-Car SoCs and only supply  the pin
> groups/functions existing on a given SoC...
>
> Signed-off-by: Sergei Shtylyov 
> Acked-by: Rob Herring 

Thank you!

Queuing in sh-pfc-for-v4.13 after dropping the annotations, as they are
implied by pin groups/functions

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3] pinctrl: sh-pfc: r8a7794: add R8A7745 support

2017-05-16 Thread Geert Uytterhoeven

Hi Sergei,

On Fri, Apr 28, 2017 at 8:52 PM, Sergei Shtylyov
 wrote:
> Renesas RZ/G1E (R8A7745) is pin compatible with R-Car E2 (R8A7794),
> however it doesn't have several automotive specific peripherals.
> Annotate all the items that only exist on the R-Car SoCs...
>
> Signed-off-by: Sergei Shtylyov 

Thank you!

Queuing in sh-pfc-for-v4.13 after dropping the annotations, as they are
implied by pin groups/functions

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [RFC PATCH v2 1/4] media: i2c: adv748x: add adv748x driver

2017-05-16 Thread Sakari Ailus

Hi Kieran,

On Mon, May 15, 2017 at 01:32:41PM +0100, Kieran Bingham wrote:
> Hi Sakari
> 
> On 12/05/17 17:46, Sakari Ailus wrote:
> > Hi Kieran,
> > 
> > Thanks for the patches!
> 
> Thankyou for the review!

You're welcome! :-)

> 
> > Would you have a media-ctl -p && media-ctl --print-dot (or the PS file) to
> > see how this looks like in practice?
> 
> The file I linked to on Friday showed the immutable links version (I have that
> in as a temporary workaround until the links are correctly ignored by rvin /
> handled by core correctly)
> 
> So again, here is the posted 'immutable' version, which represents this patch
> version:
> 
> http://janet.linuxembedded.co.uk/adv748x-immutable.png
> 
> But the end goal of this patchset will be the following configuration:
> 
> http://janet.linuxembedded.co.uk/adv748x-non-immutable.png

What does media-ctl -p say?

...

> >> +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 *status)
> >> +{
> >> +  struct adv748x_state *state = adv748x_afe_to_state(sd);
> >> +  int ret;
> >> +
> >> +  ret = mutex_lock_interruptible(&state->mutex);
> > 
> > I wonder if this is necessary. Do you expect the mutex to be taken for
> > extended periods of time?
> 
> It looks like the only other adv* driver to take a lock here is the 7180.
> Perhaps that is where the heritage of this code derived from.
> 
> I don't think the locking should be held for a long time anywhere, but I will
> try to go through and consider all the locking throughout the code base.
> 
> At the moment I think anything that calls adv748x_afe_status() should be 
> locked
> to ensure serialisation through adv748x_afe_read_ro_map().

I meant whether you need the "_interruptible" part. It's quite a bit of
repeating error handling that looks mostly unnecessary.

> 
> 
> >> +  if (ret)
> >> +  return ret;
> >> +
> >> +  ret = adv748x_afe_status(state, status, NULL);
> >> +
> >> +  mutex_unlock(&state->mutex);
> >> +  return ret;
> >> +}

...

> >> +static int adv748x_afe_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> >> +{
> >> +  struct adv748x_state *state =
> >> +  container_of(ctrl->handler, struct adv748x_state, afe.ctrl_hdl);
> >> +  unsigned int width, height, fps;
> >> +  v4l2_std_id std;
> >> +
> >> +  switch (ctrl->id) {
> >> +  case V4L2_CID_PIXEL_RATE:
> > 
> > I presume you will know when PIXEL_RATE changes? 
> 
> No interrupt handling in here yet to handle input changes - but I'm working on
> that. At the least it could be set on format configurations.

Ack. Then I guess this is good.

> 
> 
> > You can use
> > v4l2_ctrl_s_ctrl_int64() to change it. That way control events will be
> > emitted correctly, too.
> 
> Thanks - I didn't know about those helpers. I'll try to add where applicable.
> 
> These controls are indirectly read through the CSI2 'pass-through' entity.
> 
> Do I need to add some event handling in there to update it's control value to
> propagate the events?

As long as you allow subscribing the control event I guess nothing special
is needed.

..

> >> +int adv748x_afe_probe(struct adv748x_state *state, struct device_node *ep)
> >> +{
> >> +  int ret;
> >> +  unsigned int i;
> >> +
> >> +  state->afe.streaming = false;
> >> +  state->afe.curr_norm = V4L2_STD_ALL;
> >> +
> >> +  adv748x_subdev_init(&state->afe.sd, state, &adv748x_afe_ops, "afe");
> >> +
> >> +  /* Ensure that matching is based upon the endpoint fwnodes */
> >> +  state->afe.sd.fwnode = &ep->fwnode;
> > 
> > I wonder if you really need to convert all users of async matching to use
> > endpoints --- could you opportunistically peek if the device node matches,
> > just in case? You can't have an accidental positive match anyway.
> > 
> > So, the match is now for plain fwnode pointers, and it would be:
> > 
> > return async->fwnode == fwnode ||
> > port_parent(parent(async->fwnode)) == fwnode ||
> > async->fwnode == port_parent(parent(fwnode));
> 
> 
> If we adapt the core to match against endpoint comparisons and device node
> comparisons then yes we wouldn't have to adapt everything, I.e. we wouldn't 
> have
> to change all the async devices - but we would have to adapt all the 
> bus/bridge
> drivers (those who perform the v4l2_async_notifier_register() call) as they 
> must
> register their notifier against an endpoint if they are to ever be able to
> connect to an ADV748x or other device which will rely upon multiple endpoints.

If there really is a need to, we can add a new framework helper function for
that --- as I think Niklas proposed. I'm not really sure it should be really
needed though; e.g. the omap3isp driver does without that.

...

> >> +void adv748x_subdev_init(struct v4l2_subdev *sd, struct adv748x_state 
> >> *state,
> >> +  const struct v4l2_subdev_ops *ops, const char *ident)
> >> +{
> >> +  v4l2_subdev_init(sd, ops);
> >> +  sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >> +
> >> +  /* the owner is the same as the i2c_client's driver owner */
> >> +  sd->ow

Re: [PATCH] ravb: add wake-on-lan support via magic packet

2017-05-16 Thread Niklas Söderlund

Hi,

On 2017-05-16 13:36:21 +0200, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Tue, May 16, 2017 at 1:01 PM, Simon Horman  wrote:
> > On Tue, May 16, 2017 at 11:07:34AM +0200, Geert Uytterhoeven wrote:
> >> On Tue, May 16, 2017 at 11:02 AM, Niklas Söderlund
> >>  wrote:
> >> >> > Whit all this being said I still like to withdraw this patch as I 
> >> >> > found
> >> >> > another fault with it, ravb_wol_restore() will unconditionally be 
> >> >> > called
> >> >> > while ravb_wol_setup() will only be called if netif_running(ndev). 
> >> >> > This
> >> >> > is en easy fix and I will send out a v2 once we figure out what to do
> >> >> > about the clock.
> >> >>
> >> >> The clock issue is external to the ravb driver. If it works with
> >> >> s2idle, it should
> >> >> be OK.
> >> >
> >> > Do you think it's a good idea to move ahead with a v2 of the ravb WoL
> >> > patch to fix the unrelated issue and aim for it to be picked up prior to
> >> > suspend/resume support is added to the CPG/MSSR?
> >>
> >> Sure.
> >>
> >> It can still be used on R-Car Gen2, where we're not s*d by mandatory PSCI.
> >
> > Is there some way for - e.g. the driver - to not enable WoL on Gen3 SoCs
> > until the clock issues is sorted out? I'm quite happy to enable features
> 
> "priv->chip_id != RCAR_GEN3". However, as we don't have RAVB enabled
> on any R-Car Gen2 board, its use is limited.

I agree that it's not so useful to enable this on Gen2 only.

> 
> > where they work; not so much where they don't.
> 
> Agreed.
> 
> One workaround could be to disable/enable the module clock in the WoL
> resume path, to make sure it is enabled.  Once the enable count reaches
> 0, CCF will know it's disabled, and will really enable next time.
> You may need a double disable/double enable though, without testing I
> don't know remember the enable count is 1 or 2 at that point (due to PM
> runtime).

I thought about this but it feels like such a hack I did not dare 
suggest it :-) But at the same time it would be nice to enable WoL for 
the s2idle use-case where it works. Only resume from PSCI with WoL 
enabled that is broken, and WoL in PSCI suspend will never work :-)

How about I add another patch in v2 on-top of this that adds the clock 
disable/enable hack? That way it's clear that this is a workaround and 
once we have support for suspend/resume in CPG/MSSR just that patch can 
be reverted? Or is it cleaner to fold it in to this patch with a big 
comment that this is a workaround? Or is it maybe better to hold of on 
this until CPG/MSSR supports suspend/resume?

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH v2 1/4] media: i2c: adv748x: add adv748x driver

2017-05-16 Thread Kieran Bingham

Hi Sakari,

(Gianluca, I've pulled you in on CC in respect to discussing changes you
implemented for supporting GCC < 4.4.6)

On 16/05/17 12:54, Sakari Ailus wrote:
> Hi Kieran,
> 
> On Mon, May 15, 2017 at 01:32:41PM +0100, Kieran Bingham wrote:
>> Hi Sakari
>>
>> On 12/05/17 17:46, Sakari Ailus wrote:
>>> Hi Kieran,
>>>
>>> Thanks for the patches!
>>
>> Thankyou for the review!
> 
> You're welcome! :-)
> 
>>
>>> Would you have a media-ctl -p && media-ctl --print-dot (or the PS file) to
>>> see how this looks like in practice?
>>
>> The file I linked to on Friday showed the immutable links version (I have 
>> that
>> in as a temporary workaround until the links are correctly ignored by rvin /
>> handled by core correctly)
>>
>> So again, here is the posted 'immutable' version, which represents this patch
>> version:
>>
>> http://janet.linuxembedded.co.uk/adv748x-immutable.png
>>
>> But the end goal of this patchset will be the following configuration:
>>
>> http://janet.linuxembedded.co.uk/adv748x-non-immutable.png
> 
> What does media-ctl -p say?
> 

media-ctl -p | pastebinit : http://paste.ubuntu.com/24586605/

> ...
> 
 +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 *status)
 +{
 +  struct adv748x_state *state = adv748x_afe_to_state(sd);
 +  int ret;
 +
 +  ret = mutex_lock_interruptible(&state->mutex);
>>>
>>> I wonder if this is necessary. Do you expect the mutex to be taken for
>>> extended periods of time?
>>
>> It looks like the only other adv* driver to take a lock here is the 7180.
>> Perhaps that is where the heritage of this code derived from.
>>
>> I don't think the locking should be held for a long time anywhere, but I will
>> try to go through and consider all the locking throughout the code base.
>>
>> At the moment I think anything that calls adv748x_afe_status() should be 
>> locked
>> to ensure serialisation through adv748x_afe_read_ro_map().
> 
> I meant whether you need the "_interruptible" part. It's quite a bit of
> repeating error handling that looks mostly unnecessary.
> 

Aha - I see what you mean now...

Most of these use I2C transactions though, so that would be our source of any 
delay.

If it's acceptable to expect an I2C bus to never hang, or if the I2C subsystem
can handle that, then we can chop the interruptible ... but I'm not sure I2C bus
transactions are guaranteed like that ? Don't things like clock stretching, and
unreliable hardware leave us vulnerable there...

>>
>>
 +  if (ret)
 +  return ret;
 +
 +  ret = adv748x_afe_status(state, status, NULL);
 +
 +  mutex_unlock(&state->mutex);
 +  return ret;
 +}
> 
> ...
> 
 +static int adv748x_afe_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 +{
 +  struct adv748x_state *state =
 +  container_of(ctrl->handler, struct adv748x_state, afe.ctrl_hdl);
 +  unsigned int width, height, fps;
 +  v4l2_std_id std;
 +
 +  switch (ctrl->id) {
 +  case V4L2_CID_PIXEL_RATE:
>>>
>>> I presume you will know when PIXEL_RATE changes? 
>>
>> No interrupt handling in here yet to handle input changes - but I'm working 
>> on
>> that. At the least it could be set on format configurations.
> 
> Ack. Then I guess this is good.
> 
>>
>>
>>> You can use
>>> v4l2_ctrl_s_ctrl_int64() to change it. That way control events will be
>>> emitted correctly, too.
>>
>> Thanks - I didn't know about those helpers. I'll try to add where applicable.
>>
>> These controls are indirectly read through the CSI2 'pass-through' entity.
>>
>> Do I need to add some event handling in there to update it's control value to
>> propagate the events?
> 
> As long as you allow subscribing the control event I guess nothing special
> is needed.
> 
> ..
> 
 +int adv748x_afe_probe(struct adv748x_state *state, struct device_node *ep)
 +{
 +  int ret;
 +  unsigned int i;
 +
 +  state->afe.streaming = false;
 +  state->afe.curr_norm = V4L2_STD_ALL;
 +
 +  adv748x_subdev_init(&state->afe.sd, state, &adv748x_afe_ops, "afe");
 +
 +  /* Ensure that matching is based upon the endpoint fwnodes */
 +  state->afe.sd.fwnode = &ep->fwnode;
>>>
>>> I wonder if you really need to convert all users of async matching to use
>>> endpoints --- could you opportunistically peek if the device node matches,
>>> just in case? You can't have an accidental positive match anyway.
>>>
>>> So, the match is now for plain fwnode pointers, and it would be:
>>>
>>> return async->fwnode == fwnode ||
>>> port_parent(parent(async->fwnode)) == fwnode ||
>>> async->fwnode == port_parent(parent(fwnode));
>>
>>
>> If we adapt the core to match against endpoint comparisons and device node
>> comparisons then yes we wouldn't have to adapt everything, I.e. we wouldn't 
>> have
>> to change all the async devices - but we would have to adapt all the 
>> bus/bridge
>> drivers (those who perform the v4l2_async_notifier_register() call) as they

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread sricharan


Hi Laurent,

On 2017-05-16 12:47, Laurent Pinchart wrote:

Hi Sricharan,

On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:

On 2017-05-16 03:04, Laurent Pinchart wrote:
> On Monday 15 May 2017 23:37:16 Laurent Pinchart wrote:
>> On Wednesday 03 May 2017 15:54:59 Sricharan R wrote:
>>> On 5/3/2017 3:24 PM, Robin Murphy wrote:
 On 02/05/17 19:35, Geert Uytterhoeven wrote:
> On Fri, Feb 3, 2017 at 4:48 PM, Sricharan R wrote:
>> From: Laurent Pinchart 
>>
>> Failures to look up an IOMMU when parsing the DT iommus property
>> need to be handled separately from the .of_xlate() failures to
>> support deferred probing.
>>
>> The lack of a registered IOMMU can be caused by the lack of a driver
>> for the IOMMU, the IOMMU device probe not having been performed yet,
>> having been deferred, or having failed.
>>
>> The first case occurs when the device tree describes the bus master
>> and IOMMU topology correctly but no device driver exists for the
>> IOMMU yet or the device driver has not been compiled in. Return NULL,
>> the caller will configure the device without an IOMMU.
>>
>> The second and third cases are handled by deferring the probe of the
>> bus master device which will eventually get reprobed after the
>> IOMMU.
>>
>> The last case is currently handled by deferring the probe of the bus
>> master device as well. A mechanism to either configure the bus
>> master device without an IOMMU or to fail the bus master device probe
>> depending on whether the IOMMU is optional or mandatory would be a
>> good enhancement.
>>
>> Tested-by: Marek Szyprowski 
>> Signed-off-by: Laurent Pichart
>> 
>> Signed-off-by: Sricharan R 
>
> This patch broke Renesas R-Car Gen3 platforms in renesas-drivers.
> As the IOMMU nodes in DT are not yet enabled, all devices having
> iommus properties in DT now fail to probe.

 How exactly do they fail to probe? Per d7b0558230e4, if there are no
 ops registered then they should merely defer until we reach the point
 of giving up and ignoring the IOMMU. Is it just that you have no other
 late-probing drivers or post-init module loads to kick the deferred
 queue after that point? I did try to find a way to explicitly kick it
 from a suitably late initcall, but there didn't seem to be any obvious
 public interface - anyone have any suggestions?

 I think that's more of a general problem with the probe deferral
 mechanism itself (I've seen the same thing happen with some of the
 CoreSight stuff on Juno due to the number of inter-component
 dependencies) rather than any specific fault of this series.
>>>
>>> I was thinking of an additional check like below to avoid the
>>> situation ?
>>>
>>> From 499b6e662f60f23740b8880882b0a16f16434501 Mon Sep 17 00:00:00 2001
>>> From: Sricharan R 
>>> Date: Wed, 3 May 2017 13:16:59 +0530
>>> Subject: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER
>>>
>>> While returning EPROBE_DEFER for iommu masters
>>> take in to account of iommu nodes that could be
>>> marked in DT as 'status=disabled', in which case
>>> simply return NULL and let the master's probe
>>> continue rather than deferring.
>>>
>>> Signed-off-by: Sricharan R 
>>> ---
>>>
>>>  drivers/iommu/of_iommu.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>> index 9f44ee8..e6e9bec 100644
>>> --- a/drivers/iommu/of_iommu.c
>>> +++ b/drivers/iommu/of_iommu.c
>>> @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct
>>> device_node *np)
>>>
>>> ops = iommu_ops_from_fwnode(fwnode);
>>> if ((ops && !ops->of_xlate) ||
>>> +   !of_device_is_available(iommu_spec->np) ||
>>> (!ops && !of_iommu_driver_present(iommu_spec->np)))
>>> return NULL;
>>
>> This looks good to me, but won't be enough. The ipmmu-vmsa driver in
>> v4.12-rc1 doesn't call iommu_device_register() and thus won't be found
>> by iommu_ops_from_fwnode(). Furthermore, it doesn't IOMMU_OF_DECLARE(),
>> and thus will always be considered as absent.
>>
>> I agree that the ipmmu-vmsa driver needs to be fixed, but it would
>> have been nice to check existing IOMMU drivers before merging this patch
>> series...
>
> Please pardon the question, but has this patch series been tested on
> ARM32 ?
>
> When the device is probed the arch_setup_dma_ops() function is called.
> It sets the device's dma_ops and the mapping (in
> __arm_iommu_attach_device()). If probe is deferred,
> arch_teardown_dma_ops() is called which in turn calls
> arch_teardown_dma_ops(). This removes the mapping but doesn't touch the
> dma_ops. The next time the device is probed, arch_setup_dma_ops() bails
> out immediately as the dma_ops are already set, leaving us with a device
> bound to IOMMU operations but with no mapping. This oopses later as s

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Robin Murphy

Hi Laurent,

On 16/05/17 08:17, Laurent Pinchart wrote:
> Hi Sricharan,
> 
> On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
>> On 2017-05-16 03:04, Laurent Pinchart wrote:
>>> On Monday 15 May 2017 23:37:16 Laurent Pinchart wrote:
 On Wednesday 03 May 2017 15:54:59 Sricharan R wrote:
> On 5/3/2017 3:24 PM, Robin Murphy wrote:
>> On 02/05/17 19:35, Geert Uytterhoeven wrote:
>>> On Fri, Feb 3, 2017 at 4:48 PM, Sricharan R wrote:
 From: Laurent Pinchart 

 Failures to look up an IOMMU when parsing the DT iommus property
 need to be handled separately from the .of_xlate() failures to
 support deferred probing.

 The lack of a registered IOMMU can be caused by the lack of a driver
 for the IOMMU, the IOMMU device probe not having been performed yet,
 having been deferred, or having failed.

 The first case occurs when the device tree describes the bus master
 and IOMMU topology correctly but no device driver exists for the
 IOMMU yet or the device driver has not been compiled in. Return NULL,
 the caller will configure the device without an IOMMU.

 The second and third cases are handled by deferring the probe of the
 bus master device which will eventually get reprobed after the
 IOMMU.

 The last case is currently handled by deferring the probe of the bus
 master device as well. A mechanism to either configure the bus
 master device without an IOMMU or to fail the bus master device probe
 depending on whether the IOMMU is optional or mandatory would be a
 good enhancement.

 Tested-by: Marek Szyprowski 
 Signed-off-by: Laurent Pichart
 
 Signed-off-by: Sricharan R 
>>>
>>> This patch broke Renesas R-Car Gen3 platforms in renesas-drivers.
>>> As the IOMMU nodes in DT are not yet enabled, all devices having
>>> iommus properties in DT now fail to probe.
>>
>> How exactly do they fail to probe? Per d7b0558230e4, if there are no
>> ops registered then they should merely defer until we reach the point
>> of giving up and ignoring the IOMMU. Is it just that you have no other
>> late-probing drivers or post-init module loads to kick the deferred
>> queue after that point? I did try to find a way to explicitly kick it
>> from a suitably late initcall, but there didn't seem to be any obvious
>> public interface - anyone have any suggestions?
>>
>> I think that's more of a general problem with the probe deferral
>> mechanism itself (I've seen the same thing happen with some of the
>> CoreSight stuff on Juno due to the number of inter-component
>> dependencies) rather than any specific fault of this series.
>
> I was thinking of an additional check like below to avoid the
> situation ?
>
> From 499b6e662f60f23740b8880882b0a16f16434501 Mon Sep 17 00:00:00 2001
> From: Sricharan R 
> Date: Wed, 3 May 2017 13:16:59 +0530
> Subject: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER
>
> While returning EPROBE_DEFER for iommu masters
> take in to account of iommu nodes that could be
> marked in DT as 'status=disabled', in which case
> simply return NULL and let the master's probe
> continue rather than deferring.
>
> Signed-off-by: Sricharan R 
> ---
>
>  drivers/iommu/of_iommu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 9f44ee8..e6e9bec 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct
> device_node *np)
>
> ops = iommu_ops_from_fwnode(fwnode);
> if ((ops && !ops->of_xlate) ||
> +   !of_device_is_available(iommu_spec->np) ||
> (!ops && !of_iommu_driver_present(iommu_spec->np)))
> return NULL;

 This looks good to me, but won't be enough. The ipmmu-vmsa driver in
 v4.12-rc1 doesn't call iommu_device_register() and thus won't be found
 by iommu_ops_from_fwnode(). Furthermore, it doesn't IOMMU_OF_DECLARE(),
 and thus will always be considered as absent.

 I agree that the ipmmu-vmsa driver needs to be fixed, but it would
 have been nice to check existing IOMMU drivers before merging this patch
 series...
>>>
>>> Please pardon the question, but has this patch series been tested on
>>> ARM32 ?
>>>
>>> When the device is probed the arch_setup_dma_ops() function is called.
>>> It sets the device's dma_ops and the mapping (in
>>> __arm_iommu_attach_device()). If probe is deferred,
>>> arch_teardown_dma_ops() is called which in turn calls
>>> arch_teardown_dma_ops(). This removes the mapping but doesn't touch the
>>> dma_ops.

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Laurent Pinchart

Hi Sricharan,

On Tuesday 16 May 2017 19:10:03 sricha...@codeaurora.org wrote:
> On 2017-05-16 12:47, Laurent Pinchart wrote:
> > On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
> >> On 2017-05-16 03:04, Laurent Pinchart wrote:
> >>> On Monday 15 May 2017 23:37:16 Laurent Pinchart wrote:
>  On Wednesday 03 May 2017 15:54:59 Sricharan R wrote:
> > On 5/3/2017 3:24 PM, Robin Murphy wrote:
> >> On 02/05/17 19:35, Geert Uytterhoeven wrote:
> >>> On Fri, Feb 3, 2017 at 4:48 PM, Sricharan R wrote:
>  From: Laurent Pinchart 
>  
>  Failures to look up an IOMMU when parsing the DT iommus property
>  need to be handled separately from the .of_xlate() failures to
>  support deferred probing.
>  
>  The lack of a registered IOMMU can be caused by the lack of a
>  driver for the IOMMU, the IOMMU device probe not having been
>  performed yet, having been deferred, or having failed.
>  
>  The first case occurs when the device tree describes the bus
>  master and IOMMU topology correctly but no device driver exists for
>  the IOMMU yet or the device driver has not been compiled in. Return
>  NULL, the caller will configure the device without an IOMMU.
>  
>  The second and third cases are handled by deferring the probe of
>  the bus master device which will eventually get reprobed after the
>  IOMMU.
>  
>  The last case is currently handled by deferring the probe of the
>  bus master device as well. A mechanism to either configure the bus
>  master device without an IOMMU or to fail the bus master device
>  probe depending on whether the IOMMU is optional or mandatory would
>  be a good enhancement.
>  
>  Tested-by: Marek Szyprowski 
>  Signed-off-by: Laurent Pichart
>  
>  Signed-off-by: Sricharan R 
> >>> 
> >>> This patch broke Renesas R-Car Gen3 platforms in renesas-drivers.
> >>> As the IOMMU nodes in DT are not yet enabled, all devices having
> >>> iommus properties in DT now fail to probe.
> >> 
> >> How exactly do they fail to probe? Per d7b0558230e4, if there are no
> >> ops registered then they should merely defer until we reach the
> >> point of giving up and ignoring the IOMMU. Is it just that you have
> >> no other late-probing drivers or post-init module loads to kick the
> >> deferred queue after that point? I did try to find a way to
> >> explicitly kick it from a suitably late initcall, but there didn't
> >> seem to be any obvious public interface - anyone have any
> >> suggestions?
> >> 
> >> I think that's more of a general problem with the probe deferral
> >> mechanism itself (I've seen the same thing happen with some of the
> >> CoreSight stuff on Juno due to the number of inter-component
> >> dependencies) rather than any specific fault of this series.
> > 
> > I was thinking of an additional check like below to avoid the
> > situation ?
> > 
> > From 499b6e662f60f23740b8880882b0a16f16434501 Mon Sep 17 00:00:00
> > 2001
> > From: Sricharan R 
> > Date: Wed, 3 May 2017 13:16:59 +0530
> > Subject: [PATCH] iommu: of: Fix check for returning EPROBE_DEFER
> > 
> > While returning EPROBE_DEFER for iommu masters
> > take in to account of iommu nodes that could be
> > marked in DT as 'status=disabled', in which case
> > simply return NULL and let the master's probe
> > continue rather than deferring.
> > 
> > Signed-off-by: Sricharan R 
> > ---
> > 
> >  drivers/iommu/of_iommu.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 9f44ee8..e6e9bec 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -118,6 +118,7 @@ static bool of_iommu_driver_present(struct
> > device_node *np)
> > 
> > ops = iommu_ops_from_fwnode(fwnode);
> > if ((ops && !ops->of_xlate) ||
> > +   !of_device_is_available(iommu_spec->np) ||
> > (!ops && !of_iommu_driver_present(iommu_spec->np)))
> > return NULL;
>  
>  This looks good to me, but won't be enough. The ipmmu-vmsa driver in
>  v4.12-rc1 doesn't call iommu_device_register() and thus won't be found
>  by iommu_ops_from_fwnode(). Furthermore, it doesn't
>  IOMMU_OF_DECLARE(),
>  and thus will always be considered as absent.
>  
>  I agree that the ipmmu-vmsa driver needs to be fixed, but it would
>  have been nice to check existing IOMMU drivers before merging this
>  patch series...
> >>> 
> >>> Please pardon the question, but has this patch series been tested on
> >>> ARM32 ?
> >>> 
> >>> When the device is probed the a

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Laurent Pinchart

Hi Robin,

On Tuesday 16 May 2017 15:04:55 Robin Murphy wrote:
> On 16/05/17 08:17, Laurent Pinchart wrote:
> > On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:

[snip]

> >> arch_teardown_dma_ops() being the inverse of arch_setup_dma_ops()
> >> ,dma_ops should be cleared in the teardown path. Otherwise
> >> this causes problem when the probe of device is retried after
> >> being deferred. The device's iommu structures are cleared
> >> after EPROBEDEFER error, but on the next try dma_ops will still
> >> be set to old value, which is not right.
> >> 
> >> Signed-off-by: Sricharan R 
> >> Reviewed-by: Robin Murphy 
> >> ---
> >> 
> >>   arch/arm/mm/dma-mapping.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> >> index ab4f745..a40f03e 100644
> >> --- a/arch/arm/mm/dma-mapping.c
> >> +++ b/arch/arm/mm/dma-mapping.c
> >> @@ -2358,6 +2358,7 @@ static void arm_teardown_iommu_dma_ops(struct
> >> device *dev)
> > 
> >>__arm_iommu_detach_device(dev);
> >>arm_iommu_release_mapping(mapping);
> >> +  set_dma_ops(dev, NULL);
> >>   }
> >>   #else
> > 
> > The subject mentions arch_teardown_dma_ops(), which I think is correct,
> > but the patch adds the set_dma_ops() call to arm_teardown_iommu_dma_ops().
> > 
> > However, the situation is perhaps more complex. Note the check at the
> > beginning of arch_setup_dma_ops():
> > /*
> >  * Don't override the dma_ops if they have already been set. Ideally
> >  * this should be the only location where dma_ops are set, remove this
> >  * check when all other callers of set_dma_ops will have disappeared.
> >  */
> > if (dev->dma_ops)
> > return;
> > 
> > If you set the dma_ops to NULL in arm_teardown_iommu_dma_ops() or
> > arch_teardown_dma_ops(), the next call to arch_setup_dma_ops() will
> > override them. To be safe you should only set them to NULL if they have
> > been set by arch_setup_dma_ops(). More than that, arch_teardown_dma_ops()
> > should probably not call arm_teardown_iommu_dma_ops() at all if the
> > dma_ops were set by arm_iommu_attach_device() and not
> > arch_teardown_dma_ops().
> 
> Under what circumstances is that an issue? We'll only be tearing down
> the DMA ops when unbinding the driver,

Or when deferring probe.

> and I think it would be erroneous to expect the device to retain much state
> after that. Everything else would be set up from scratch again if it get
> reprobed later, so why not the DMA ops?

Because the DMA ops might have been set elsewhere than arch_setup_dma_ops(). 
If you look at the patch that added the above warning, its commit message 
states

commit 26b37b946a5c2658dbc37dd5d6df40aaa9685d70 (iommu-joerg/arm/core)
Author: Laurent Pinchart 
Date:   Fri May 15 02:00:02 2015 +0300

arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

The arch_setup_dma_ops() function is in charge of setting dma_ops with a
call to set_dma_ops(). set_dma_ops() is also called from

- highbank and mvebu bus notifiers
- dmabounce (to be replaced with swiotlb)
- arm_iommu_attach_device

(arm_iommu_attach_device is itself called from IOMMU and bus master
device drivers)

To allow the arch_setup_dma_ops() call to be moved from device add time
to device probe time we must ensure that dma_ops already setup by any of
the above callers will not be overriden.

Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to
of_xlate and taking care of highbank and mvebu, the workaround should be
removed.

I'm concerned about potentially breaking these if we unconditionally remove 
the DMA ops and mapping.

-- 
Regards,

Laurent Pinchart

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread sricharan


Hi,

On 2017-05-16 19:40, Laurent Pinchart wrote:

Hi Robin,

On Tuesday 16 May 2017 15:04:55 Robin Murphy wrote:

On 16/05/17 08:17, Laurent Pinchart wrote:
> On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:


[snip]


>> arch_teardown_dma_ops() being the inverse of arch_setup_dma_ops()
>> ,dma_ops should be cleared in the teardown path. Otherwise
>> this causes problem when the probe of device is retried after
>> being deferred. The device's iommu structures are cleared
>> after EPROBEDEFER error, but on the next try dma_ops will still
>> be set to old value, which is not right.
>>
>> Signed-off-by: Sricharan R 
>> Reviewed-by: Robin Murphy 
>> ---
>>
>>   arch/arm/mm/dma-mapping.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ab4f745..a40f03e 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -2358,6 +2358,7 @@ static void arm_teardown_iommu_dma_ops(struct
>> device *dev)
>
>>__arm_iommu_detach_device(dev);
>>arm_iommu_release_mapping(mapping);
>> +  set_dma_ops(dev, NULL);
>>   }
>>   #else
>
> The subject mentions arch_teardown_dma_ops(), which I think is correct,
> but the patch adds the set_dma_ops() call to arm_teardown_iommu_dma_ops().
>
> However, the situation is perhaps more complex. Note the check at the
> beginning of arch_setup_dma_ops():
>/*
> * Don't override the dma_ops if they have already been set. Ideally
> * this should be the only location where dma_ops are set, remove this
> * check when all other callers of set_dma_ops will have disappeared.
> */
>if (dev->dma_ops)
>return;
>
> If you set the dma_ops to NULL in arm_teardown_iommu_dma_ops() or
> arch_teardown_dma_ops(), the next call to arch_setup_dma_ops() will
> override them. To be safe you should only set them to NULL if they have
> been set by arch_setup_dma_ops(). More than that, arch_teardown_dma_ops()
> should probably not call arm_teardown_iommu_dma_ops() at all if the
> dma_ops were set by arm_iommu_attach_device() and not
> arch_teardown_dma_ops().

Under what circumstances is that an issue? We'll only be tearing down
the DMA ops when unbinding the driver,


Or when deferring probe.

and I think it would be erroneous to expect the device to retain much 
state
after that. Everything else would be set up from scratch again if it 
get

reprobed later, so why not the DMA ops?


Because the DMA ops might have been set elsewhere than 
arch_setup_dma_ops().
If you look at the patch that added the above warning, its commit 
message

states

commit 26b37b946a5c2658dbc37dd5d6df40aaa9685d70 (iommu-joerg/arm/core)
Author: Laurent Pinchart 
Date:   Fri May 15 02:00:02 2015 +0300

arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

The arch_setup_dma_ops() function is in charge of setting dma_ops 
with a

call to set_dma_ops(). set_dma_ops() is also called from

- highbank and mvebu bus notifiers
- dmabounce (to be replaced with swiotlb)
- arm_iommu_attach_device

(arm_iommu_attach_device is itself called from IOMMU and bus master
device drivers)

To allow the arch_setup_dma_ops() call to be moved from device add 
time
to device probe time we must ensure that dma_ops already setup by 
any of

the above callers will not be overriden.

Aftering replacing dmabounce with swiotlb, converting IOMMU drivers 
to
of_xlate and taking care of highbank and mvebu, the workaround 
should be

removed.

I'm concerned about potentially breaking these if we unconditionally 
remove

the DMA ops and mapping.


arch_teardown_dma_ops does nothing if there is
no mapping (not behind iommu), dma_ops without iommu is ok.
But when the arm_iommu_create_mapping/arm_iommu_attach_device
was called previously in the iommu driver, after we teardown,
that path in the iommu driver which called those functions is not
replayed.

Regards,

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Laurent Pinchart

Hi Sricharan,

On Tuesday 16 May 2017 19:59:01 sricha...@codeaurora.org wrote:
> On 2017-05-16 19:40, Laurent Pinchart wrote:
> > On Tuesday 16 May 2017 15:04:55 Robin Murphy wrote:
> >> On 16/05/17 08:17, Laurent Pinchart wrote:
> >> > On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
> > [snip]
> > 
>  arch_teardown_dma_ops() being the inverse of arch_setup_dma_ops(),
>  dma_ops should be cleared in the teardown path. Otherwise
>  this causes problem when the probe of device is retried after
>  being deferred. The device's iommu structures are cleared
>  after EPROBEDEFER error, but on the next try dma_ops will still
>  be set to old value, which is not right.
>  
>  Signed-off-by: Sricharan R 
>  Reviewed-by: Robin Murphy 
>  ---
>  
>    arch/arm/mm/dma-mapping.c | 1 +
>    1 file changed, 1 insertion(+)
>  
>  diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>  index ab4f745..a40f03e 100644
>  --- a/arch/arm/mm/dma-mapping.c
>  +++ b/arch/arm/mm/dma-mapping.c
>  @@ -2358,6 +2358,7 @@ static void arm_teardown_iommu_dma_ops(struct
>  device *dev)
> 
>   __arm_iommu_detach_device(dev);
>   arm_iommu_release_mapping(mapping);
>  +set_dma_ops(dev, NULL);
>    }
>    #else
> >>> 
> >>> The subject mentions arch_teardown_dma_ops(), which I think is correct,
> >>> but the patch adds the set_dma_ops() call to
> >>> arm_teardown_iommu_dma_ops().
> >>> 
> >>> However, the situation is perhaps more complex. Note the check at the
> >>> beginning of arch_setup_dma_ops():
> >>> 
> >>>   /*
> >>>* Don't override the dma_ops if they have already been set. Ideally
> >>>* this should be the only location where dma_ops are set, remove this
> >>>* check when all other callers of set_dma_ops will have disappeared.
> >>>*/
> >>>   if (dev->dma_ops)
> >>>   return;
> >>> 
> >>> If you set the dma_ops to NULL in arm_teardown_iommu_dma_ops() or
> >>> arch_teardown_dma_ops(), the next call to arch_setup_dma_ops() will
> >>> override them. To be safe you should only set them to NULL if they have
> >>> been set by arch_setup_dma_ops(). More than that,
> >>> arch_teardown_dma_ops()
> >>> should probably not call arm_teardown_iommu_dma_ops() at all if the
> >>> dma_ops were set by arm_iommu_attach_device() and not
> >>> arch_teardown_dma_ops().
> >> 
> >> Under what circumstances is that an issue? We'll only be tearing down
> >> the DMA ops when unbinding the driver,
> > 
> > Or when deferring probe.
> > 
> >> and I think it would be erroneous to expect the device to retain much
> >> state after that. Everything else would be set up from scratch again if
> >> it get reprobed later, so why not the DMA ops?
> > 
> > Because the DMA ops might have been set elsewhere than
> > arch_setup_dma_ops(). If you look at the patch that added the above
> > warning, its commit message states
> > 
> > commit 26b37b946a5c2658dbc37dd5d6df40aaa9685d70 (iommu-joerg/arm/core)
> > Author: Laurent Pinchart 
> > Date:   Fri May 15 02:00:02 2015 +0300
> > 
> > arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
> > 
> > The arch_setup_dma_ops() function is in charge of setting dma_ops
> > with a call to set_dma_ops(). set_dma_ops() is also called from
> > 
> > - highbank and mvebu bus notifiers
> > - dmabounce (to be replaced with swiotlb)
> > - arm_iommu_attach_device
> > 
> > (arm_iommu_attach_device is itself called from IOMMU and bus master
> > device drivers)
> > 
> > To allow the arch_setup_dma_ops() call to be moved from device add
> > time to device probe time we must ensure that dma_ops already setup by
> > any of the above callers will not be overriden.
> > 
> > Aftering replacing dmabounce with swiotlb, converting IOMMU drivers
> > to of_xlate and taking care of highbank and mvebu, the workaround
> > should be removed.
> > 
> > I'm concerned about potentially breaking these if we unconditionally
> > remove the DMA ops and mapping.
> 
> arch_teardown_dma_ops does nothing if there is no mapping (not behind
> iommu), dma_ops without iommu is ok. But when the
> arm_iommu_create_mapping/arm_iommu_attach_device was called previously in
> the iommu driver, after we teardown, that path in the iommu driver which
> called those functions is not replayed.

I've had a look at the code in more details, and I'm not sure how we've 
reached the current situation (as I haven't followed the multiple versions of 
this patch series due to lack of time), but it's a very big mess.

arch_setup_dma_ops() is currently not the only way to create a mapping and 
attach it to a device. Not only that, but with the current ARM32 dma-mapping 
implementation, it can't be.

arch_setup_dma_ops() will create a separate mapping for every device. That's 
certainly fine when every device has its own IOMMU instance (

Re: [PATCH V8 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2017-05-16 Thread Robin Murphy

On 16/05/17 15:10, Laurent Pinchart wrote:
> Hi Robin,
> 
> On Tuesday 16 May 2017 15:04:55 Robin Murphy wrote:
>> On 16/05/17 08:17, Laurent Pinchart wrote:
>>> On Tuesday 16 May 2017 07:53:57 sricha...@codeaurora.org wrote:
> 
> [snip]
> 
 arch_teardown_dma_ops() being the inverse of arch_setup_dma_ops()
 ,dma_ops should be cleared in the teardown path. Otherwise
 this causes problem when the probe of device is retried after
 being deferred. The device's iommu structures are cleared
 after EPROBEDEFER error, but on the next try dma_ops will still
 be set to old value, which is not right.

 Signed-off-by: Sricharan R 
 Reviewed-by: Robin Murphy 
 ---

   arch/arm/mm/dma-mapping.c | 1 +
   1 file changed, 1 insertion(+)

 diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
 index ab4f745..a40f03e 100644
 --- a/arch/arm/mm/dma-mapping.c
 +++ b/arch/arm/mm/dma-mapping.c
 @@ -2358,6 +2358,7 @@ static void arm_teardown_iommu_dma_ops(struct
 device *dev)
>>>
__arm_iommu_detach_device(dev);
arm_iommu_release_mapping(mapping);
 +  set_dma_ops(dev, NULL);
   }
   #else
>>>
>>> The subject mentions arch_teardown_dma_ops(), which I think is correct,
>>> but the patch adds the set_dma_ops() call to arm_teardown_iommu_dma_ops().
>>>
>>> However, the situation is perhaps more complex. Note the check at the
>>> beginning of arch_setup_dma_ops():
>>> /*
>>>  * Don't override the dma_ops if they have already been set. Ideally
>>>  * this should be the only location where dma_ops are set, remove this
>>>  * check when all other callers of set_dma_ops will have disappeared.
>>>  */
>>> if (dev->dma_ops)
>>> return;
>>>
>>> If you set the dma_ops to NULL in arm_teardown_iommu_dma_ops() or
>>> arch_teardown_dma_ops(), the next call to arch_setup_dma_ops() will
>>> override them. To be safe you should only set them to NULL if they have
>>> been set by arch_setup_dma_ops(). More than that, arch_teardown_dma_ops()
>>> should probably not call arm_teardown_iommu_dma_ops() at all if the
>>> dma_ops were set by arm_iommu_attach_device() and not
>>> arch_teardown_dma_ops().
>>
>> Under what circumstances is that an issue? We'll only be tearing down
>> the DMA ops when unbinding the driver,
> 
> Or when deferring probe.
> 
>> and I think it would be erroneous to expect the device to retain much state
>> after that. Everything else would be set up from scratch again if it get
>> reprobed later, so why not the DMA ops?
> 
> Because the DMA ops might have been set elsewhere than arch_setup_dma_ops(). 
> If you look at the patch that added the above warning, its commit message 
> states
> 
> commit 26b37b946a5c2658dbc37dd5d6df40aaa9685d70 (iommu-joerg/arm/core)
> Author: Laurent Pinchart 
> Date:   Fri May 15 02:00:02 2015 +0300
> 
> arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()
> 
> The arch_setup_dma_ops() function is in charge of setting dma_ops with a
> call to set_dma_ops(). set_dma_ops() is also called from
> 
> - highbank and mvebu bus notifiers
> - dmabounce (to be replaced with swiotlb)
> - arm_iommu_attach_device
> 
> (arm_iommu_attach_device is itself called from IOMMU and bus master
> device drivers)
> 
> To allow the arch_setup_dma_ops() call to be moved from device add time
> to device probe time we must ensure that dma_ops already setup by any of
> the above callers will not be overriden.
> 
> Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to
> of_xlate and taking care of highbank and mvebu, the workaround should be
> removed.
> 
> I'm concerned about potentially breaking these if we unconditionally remove 
> the DMA ops and mapping.

Ah, sorry, I see now - it was taking a long time to page the 32-bit code
back in, and I'd forgotten the specifics of the mess of competing
"default domain" notions. Indeed, it's not the device's driver expecting
any state to be preserved as I got stuck on, it's the IOMMU driver,
which does "know better" to an extent, expecting its changes to the
struct device to stick for the lifetime of that structure.

I agree there shouldn't be a disparity - arch_setup_dma_ops() only does
things given certain circumstances, so arch_teardown_dma_ops() should
only undo them under the same. With probe-deferral in place I'll be
reviving my work to convert this path over to IOMMU API default domains,
which will make some of these issues go away again, but in the meantime
I also agree that the most expedient fix is indeed to add a flag to say
whether the dma ops were automatically set or not (this is implicitly
true on arm64, which was partly what was tripping me up).

Robin.

Re: ASoC: rsnd: fix sound route path when using SRC6/SRC9

2017-05-16 Thread Geert Uytterhoeven

Hi Morimoto-san,

On Fri, Mar 31, 2017 at 9:32 PM, Linux Kernel Mailing List
 wrote:
> Web:
> https://git.kernel.org/torvalds/c/a1c2ff53726907aff5feb37e4cfd45c1ff626431
> Commit: a1c2ff53726907aff5feb37e4cfd45c1ff626431
> Parent: 4b30eebfc35c67771b5f58d9274d3e321b72d7a8
> Refname:refs/heads/master
> Author: Hiroyuki Yokoyama 
> AuthorDate: Wed Mar 1 03:51:00 2017 +
> Committer:  Mark Brown 
> CommitDate: Mon Mar 13 12:58:07 2017 +
>
> ASoC: rsnd: fix sound route path when using SRC6/SRC9
>
> This patch fixes the problem that the missing value of the route path
> setting table and incorrect values are set in the CMD_ROUTE_SELECT
> register.
>
> Signed-off-by: Hiroyuki Yokoyama 
> [Kuninori: shared data on MIX and non-MIX case]
> Signed-off-by: Kuninori Morimoto 
> Signed-off-by: Mark Brown 
> ---
>  sound/soc/sh/rcar/cmd.c | 36 
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/sound/soc/sh/rcar/cmd.c b/sound/soc/sh/rcar/cmd.c
> index abb5eaac854a..7d92a24b7cfa 100644
> --- a/sound/soc/sh/rcar/cmd.c
> +++ b/sound/soc/sh/rcar/cmd.c
> @@ -31,23 +31,24 @@ static int rsnd_cmd_init(struct rsnd_mod *mod,
> struct rsnd_mod *mix = rsnd_io_to_mod_mix(io);
> struct device *dev = rsnd_priv_to_dev(priv);
> u32 data;
> +   u32 path[] = {
> +   [1] = 1 << 0,
> +   [5] = 1 << 8,
> +   [6] = 1 << 12,
> +   [9] = 1 << 15,
> +   };
>
> if (!mix && !dvc)
> return 0;
>
> +   if (ARRAY_SIZE(path) < rsnd_mod_id(mod) + 1)
> +   return -ENXIO;
> +
> if (mix) {
> struct rsnd_dai *rdai;
> struct rsnd_mod *src;
> struct rsnd_dai_stream *tio;
> int i;
> -   u32 path[] = {
> -   [0] = 0,
> -   [1] = 1 << 0,
> -   [2] = 0,
> -   [3] = 0,
> -   [4] = 0,
> -   [5] = 1 << 8
> -   };
>
> /*
>  * it is assuming that integrater is well understanding about
> @@ -70,16 +71,19 @@ static int rsnd_cmd_init(struct rsnd_mod *mod,
> } else {
> struct rsnd_mod *src = rsnd_io_to_mod_src(io);
>
> -   u32 path[] = {
> -   [0] = 0x3,
> -   [1] = 0x30001,
> -   [2] = 0x4,
> -   [3] = 0x1,
> -   [4] = 0x2,
> -   [5] = 0x40100
> +   u8 cmd_case[] = {
> +   [0] = 0x3,
> +   [1] = 0x3,
> +   [2] = 0x4,
> +   [3] = 0x1,
> +   [4] = 0x2,
> +   [5] = 0x4,
> +   [6] = 0x1,
> +   [9] = 0x2,
> };
>
> -   data = path[rsnd_mod_id(src)];
> +   data = path[rsnd_mod_id(src)] |

With gcc 4.9.0:

sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init':
sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below
array bounds [-Warray-bounds]
   data = path[rsnd_mod_id(src)] |

> +   cmd_case[rsnd_mod_id(src)] << 16;
> }
>
> dev_dbg(dev, "ctu/mix path = 0x%08x", data);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH] ARM: dma-mapping: Don't tear third-party mappings

2017-05-16 Thread Laurent Pinchart

arch_setup_dma_ops() is used in device probe code paths to create an
IOMMU mapping and attach it to the device. The function assumes that the
device is attached to a device-specific IOMMU instance (or at least a
device-specific TLB in a shared IOMMU instance) and thus creates a
separate mapping for every device.

On several systems (Renesas R-Car Gen2 being one of them), that
assumption is not true, and IOMMU mappings must be shared between
multiple devices. In those cases the IOMMU driver knows better than the
generic ARM dma-mapping layer and attaches mapping to devices manually
with arm_iommu_attach_device(), which sets the DMA ops for the device.

The arch_setup_dma_ops() function takes this into account and bails out
immediately if the device already has DMA ops assigned. However, the
corresponding arch_teardown_dma_ops() function, called from driver
unbind code paths (including probe deferral), will tear the mapping down
regardless of who created it. When the device is reprobed
arch_setup_dma_ops() will be called again but won't perform any
operation as the DMA ops will still be set.

We need to reset the DMA ops in arch_teardown_dma_ops() to fix this.
However, we can't do so unconditionally, as then a new mapping would be
created by arch_setup_dma_ops() when the device is reprobed, regardless
of whether the device needs to share a mapping or not. We must thus keep
track of whether arch_setup_dma_ops() created the mapping, and only in
that case tear it down in arch_teardown_dma_ops().

Keep track of that information in the dev_archdata structure. As the
structure is embedded in all instances of struct device let's not grow
it, but turn the existing dma_coherent bool field into a bitfield that
can be used for other purposes.

Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred 
probing or error")
Signed-off-by: Laurent Pinchart 
---
 arch/arm/include/asm/device.h | 3 ++-
 arch/arm/mm/dma-mapping.c | 5 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index 36ec9c8f6e16..3234fe9bba6e 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -19,7 +19,8 @@ struct dev_archdata {
 #ifdef CONFIG_XEN
const struct dma_map_ops *dev_dma_ops;
 #endif
-   bool dma_coherent;
+   unsigned int dma_coherent:1;
+   unsigned int dma_ops_setup:1;
 };
 
 struct omap_device;
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c742dfd2967b..e0272f9140e2 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2430,9 +2430,14 @@ void arch_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
dev->dma_ops = xen_dma_ops;
}
 #endif
+   dev->archdata.dma_ops_setup = true;
 }
 
 void arch_teardown_dma_ops(struct device *dev)
 {
+   if (!dev->archdata.dma_ops_setup)
+   return;
+
arm_teardown_iommu_dma_ops(dev);
+   set_dma_ops(dev, NULL);
 }
-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/2] v4l: vsp1: Provide a writeback video device

2017-05-16 Thread Geert Uytterhoeven

Hi Kieran,

On Tue, May 9, 2017 at 6:39 PM, Kieran Bingham
 wrote:
> When the VSP1 is used in an active display pipeline, the output of the
> WPF can supply the LIF entity directly and simultaneously write to
> memory.
>
> Support this functionality in the VSP1 driver, by extending the WPF
> source pads, and establishing a V4L2 video device node connected to the
> new source.
>
> The source will be able to perform pixel format conversion, but not
> rescaling, and as such the output from the memory node will always be
> of the same dimensions as the display output.
>
> Signed-off-by: Kieran Bingham 

> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c

> @@ -900,6 +901,147 @@ static const struct vb2_ops vsp1_video_queue_qops = {
> .stop_streaming = vsp1_video_stop_streaming,
>  };
>
> +
> +/* 
> -
> + * videobuf2 queue operations for writeback nodes
> + */
> +
> +static void vsp1_video_wb_process_buffer(struct vsp1_video *video)
> +{
> +   struct vsp1_vb2_buffer *buf;
> +   unsigned long flags;
> +
> +   /*
> +* Writeback uses a running stream, unlike the M2M interface which
> +* controls a pipeline process manually though the use of
> +* vsp1_pipeline_run().
> +*
> +* Instead writeback will commence at the next frame interval, and can
> +* be marked complete at the interval following that. To handle this 
> we
> +* store the configured buffer as pending until the next callback.
> +*
> +* |||||
> +*  A   |<-->|
> +*   B   |<-->|
> +*C   |<-->| : Only at interrupt C can A be marked done
> +*/
> +
> +   spin_lock_irqsave(&video->irqlock, flags);
> +
> +   /* Move the pending image to the active hw queue */
> +   if (video->pending) {
> +   list_add_tail(&video->pending->queue, &video->irqqueue);
> +   video->pending = NULL;
> +   }
> +
> +   buf = list_first_entry_or_null(&video->wbqueue, struct 
> vsp1_vb2_buffer,
> +   queue);
> +
> +   if (buf) {
> +   video->rwpf->mem = buf->mem;
> +
> +   /*
> +* Store this buffer as pending. It will commence at the next
> +* frame start interrupt
> +*/
> +   video->pending = buf;
> +   list_del(&buf->queue);
> +   } else {
> +   /* Disable writeback with no buffer */
> +   video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };

With gcc 4.9.0:

drivers/media/platform/vsp1/vsp1_video.c: In function
'vsp1_video_wb_process_buffer':
drivers/media/platform/vsp1/vsp1_video.c:942:30: warning: missing
braces around initializer [-Wmissing-braces]
   video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };

-   video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
+   video->rwpf->mem = (struct vsp1_rwpf_memory) { { 0, } };

> +static void vsp1_video_wb_stop_streaming(struct vb2_queue *vq)
> +{
> +   struct vsp1_video *video = vb2_get_drv_priv(vq);
> +   struct vsp1_rwpf *rwpf = video->rwpf;
> +   struct vsp1_pipeline *pipe = rwpf->pipe;
> +   struct vsp1_vb2_buffer *buffer;
> +   unsigned long flags;
> +
> +   /*
> +* Disable the completion interrupts, and clear the WPF memory to
> +* prevent writing out frames
> +*/
> +   spin_lock_irqsave(&video->irqlock, flags);
> +   video->frame_end = NULL;
> +   rwpf->mem = (struct vsp1_rwpf_memory) { 0 };

Likewise:

-   rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
+   rwpf->mem = (struct vsp1_rwpf_memory) { { 0, } };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: dma-mapping: Don't tear third-party mappings

2017-05-16 Thread Robin Murphy

On 16/05/17 16:14, Laurent Pinchart wrote:
> arch_setup_dma_ops() is used in device probe code paths to create an
> IOMMU mapping and attach it to the device. The function assumes that the
> device is attached to a device-specific IOMMU instance (or at least a
> device-specific TLB in a shared IOMMU instance) and thus creates a
> separate mapping for every device.
> 
> On several systems (Renesas R-Car Gen2 being one of them), that
> assumption is not true, and IOMMU mappings must be shared between
> multiple devices. In those cases the IOMMU driver knows better than the
> generic ARM dma-mapping layer and attaches mapping to devices manually
> with arm_iommu_attach_device(), which sets the DMA ops for the device.
> 
> The arch_setup_dma_ops() function takes this into account and bails out
> immediately if the device already has DMA ops assigned. However, the
> corresponding arch_teardown_dma_ops() function, called from driver
> unbind code paths (including probe deferral), will tear the mapping down
> regardless of who created it. When the device is reprobed
> arch_setup_dma_ops() will be called again but won't perform any
> operation as the DMA ops will still be set.
> 
> We need to reset the DMA ops in arch_teardown_dma_ops() to fix this.
> However, we can't do so unconditionally, as then a new mapping would be
> created by arch_setup_dma_ops() when the device is reprobed, regardless
> of whether the device needs to share a mapping or not. We must thus keep
> track of whether arch_setup_dma_ops() created the mapping, and only in
> that case tear it down in arch_teardown_dma_ops().
> 
> Keep track of that information in the dev_archdata structure. As the
> structure is embedded in all instances of struct device let's not grow
> it, but turn the existing dma_coherent bool field into a bitfield that
> can be used for other purposes.
> 
> Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred 
> probing or error")
> Signed-off-by: Laurent Pinchart 
> ---
>  arch/arm/include/asm/device.h | 3 ++-
>  arch/arm/mm/dma-mapping.c | 5 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> index 36ec9c8f6e16..3234fe9bba6e 100644
> --- a/arch/arm/include/asm/device.h
> +++ b/arch/arm/include/asm/device.h
> @@ -19,7 +19,8 @@ struct dev_archdata {
>  #ifdef CONFIG_XEN
>   const struct dma_map_ops *dev_dma_ops;
>  #endif
> - bool dma_coherent;
> + unsigned int dma_coherent:1;

This should only ever be accessed by the Xen DMA code via the
is_device_dma_coherent() helper, so I can't see the change of storage
type causing any problems.

> + unsigned int dma_ops_setup:1;
>  };
>  
>  struct omap_device;
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index c742dfd2967b..e0272f9140e2 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2430,9 +2430,14 @@ void arch_setup_dma_ops(struct device *dev, u64 
> dma_base, u64 size,
>   dev->dma_ops = xen_dma_ops;
>   }
>  #endif
> + dev->archdata.dma_ops_setup = true;
>  }
>  
>  void arch_teardown_dma_ops(struct device *dev)
>  {
> + if (!dev->archdata.dma_ops_setup)
> + return;
> +
>   arm_teardown_iommu_dma_ops(dev);
> + set_dma_ops(dev, NULL);

Should we clear dma_ops_setup here for symmetry? I guess in practice
it's down to the IOMMU driver so will never change after the first
probe, but it still feels like a bit of a nagging loose end.

With that (or firm reassurance that it's OK not to),

Reviewed-by: Robin Murphy 

Apologies for being too arm64-focused in the earlier reviews and
overlooking this. Should the patch supersede 8674/1 currently in
Russell's incoming box?

Robin.

>  }
>

renesas-drivers-2017-05-16-v4.12-rc1

2017-05-16 Thread Geert Uytterhoeven

I have pushed renesas-drivers-2017-05-16-v4.12-rc1 to
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git

This tree is meant to ease development of platform support and drivers
for Renesas ARM SoCs. It is created by merging (a) the for-next branches
of various subsystem trees and (b) branches with driver code submitted
or planned for submission to maintainers into the development branch of
Simon Horman's renesas.git tree.

Today's version is based on renesas-devel-20170515v3-v4.12-rc1.

Included branches with driver code:
  - clk-renesas-for-v4.13
  - sh-pfc-for-v4.13
  - topic/rza1-pfc-gpio-v5
  - topic/ipmmu-multi-arch-v7
  - topic/r8a7795-ipmmu-v3
  - topic/r8a7796-ipmmu-v3
  - topic/r8a7795es2-dt-v4
  - git://git.ragnatech.se/linux#for-renesas-drivers
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#renesas/topic/sdhi-cmd23
  - topic/suspend-to-idle-v1
  - git://linuxtv.org/pinchartl/media.git#drm/next/du-vsp
  - topic/spi-slave-v3
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#tags/submissions/vsp-drm-race/v4
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#tags/submissions/vsp1/suspend-resume/v5
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/kbingham/rcar.git#tags/submissions/vsp1/writeback/v3
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git#topic/sdhi-refactor-v2

Included fixes:
  - iommu/ipmmu-vmsa: Fix pgsize_bitmap semicolon typo
  - serial: sh-sci: Fix race condition causing garbage during shutdown
  - iio: adc: max9611: Fix attribute measure unit

Included subsystem trees:
  - git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git#linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git#clk-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git#for-next
  - git://git.infradead.org/users/dedekind/l2-mtd-2.6.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git#tty-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git#i2c/for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git#usb-next
  - git://people.freedesktop.org/~airlied/linux#drm-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git#next
  - git://linuxtv.org/mchehab/media-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git#for-next
  - git://git.linaro.org/people/daniel.lezcano/linux.git#clockevents/next
  - git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git#testing/next
  - git://git.infradead.org/users/vkoul/slave-dma.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git#staging-next
  - git://git.armlinux.org.uk/~rmk/linux-arm.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git#for-next
  - git://git.infradead.org/users/jcooper/linux.git#irqchip/for-next
  - git://github.com/bzolnier/linux.git#fbdev-for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git#for-next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git#for-next
  - git://www.linux-watchdog.org/linux-watchdog-next.git#master
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git#for-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git#for-next/core
  - git://anongit.freedesktop.org/drm/drm-misc#for-linux-next
  - git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git#next
  - git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git#next
  - 
git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git#next

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v3 00/09] arm64: dts: r8a7795: IPMMU upstream integration V3

2017-05-16 Thread Geert Uytterhoeven

Hi Magnus,

On Mon, Mar 20, 2017 at 9:35 AM, Magnus Damm  wrote:
> arm64: dts: r8a7795: IPMMU upstream integration V3
>
> [PATCH v3 01/09] arm64: dts: r8a7795: Add IPMMU device nodes
> [PATCH v3 02/09] arm64: dts: r8a7795: Tie Audio/SYS-DMAC to IPMMU-DS0/1 and 
> MP1
> [PATCH v3 03/09] arm64: dts: r8a7795: Point DU/VSPD via FCPVD to IPMMU-VI
> [PATCH v3 04/09] arm64: dts: r8a7795: Point FDP1 via FCPF to IPMMU-VP
> [PATCH v3 05/09] arm64: dts: r8a7795: Point VSPBC/VSPBD via FCPVB to IPMMU-VP
> [PATCH v3 06/09] arm64: dts: r8a7795: Point VSPI via FCPVI to IPMMU-VP
> [PATCH v3 07/09] arm64: dts: r8a7795: Connect Ethernet-AVB to IPMMU-DS0
> [PATCH v3 08/09] arm64: dts: r8a7795: Connect SATA to IPMMU-HC
> [PATCH v3 09/09] arm64: dts: r8a7795: Enable IPMMU-VI, VP, MP1, DS0, DS1 and 
> MM
>
> This series adds DT nodes for IPMMU instances on r8a7795 together with
> connections to various r8a7795 on-chip devices such as Audio-DMAC, SYS-DMAC,
> Ethernet-AVB, SATA and a bunch of multimedia devices that make use of FCP.
>
> With these patches applied a white list enabled IPMMU driver may be used
> to check silicon revision and then enable IPMMU in the known working cases.
>
> The recommended IPMMU driver patch stack consists of the following series:
>  [PATCH v7 00/07] iommu/ipmmu-vmsa: IPMMU multi-arch update V7
>  [PATCH v3 00/09] iommu/ipmmu-vmsa: r8a7795 support V3
>  [PATCH v3 0/3] iommu/ipmmu-vmsa: r8a7796 support V3
>
> The final patch in the series enable IPMMU support for all IPMMU instances
> on r8a7795 that are used by IPMMU devices listed above with one exception.
> The exception is the SATA device connected to IPMMU-HC which still is disabled
> pending IPMMU USB integration support. I expect IPMMU USB integration to be
> handled as a second step once this series is agreed on.
>
> The DT binding for r8a7795 has since long been included in mainline
> and this series implements support following such format:

I dropped this series from today's renesas-drivers because of two reasons:
  1. IPMMU is busted in v4.12-rc1,
  2. The addition of support for R-Car H3 ES2.0 complicated the addition of
  IPMMU devices nodes and links from their slaves.

This also means I couldn't apply Morimoto-san's "[PATCH v2 --/--] arm64:
renesas: r8a7796: add IPMMU support for AUDIO DMAC", as it depends
on your series.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH] ARM: dma-mapping: Don't tear third-party mappings

2017-05-16 Thread Laurent Pinchart

Hi Robin,

On Tuesday 16 May 2017 16:47:36 Robin Murphy wrote:
> On 16/05/17 16:14, Laurent Pinchart wrote:
> > arch_setup_dma_ops() is used in device probe code paths to create an
> > IOMMU mapping and attach it to the device. The function assumes that the
> > device is attached to a device-specific IOMMU instance (or at least a
> > device-specific TLB in a shared IOMMU instance) and thus creates a
> > separate mapping for every device.
> > 
> > On several systems (Renesas R-Car Gen2 being one of them), that
> > assumption is not true, and IOMMU mappings must be shared between
> > multiple devices. In those cases the IOMMU driver knows better than the
> > generic ARM dma-mapping layer and attaches mapping to devices manually
> > with arm_iommu_attach_device(), which sets the DMA ops for the device.
> > 
> > The arch_setup_dma_ops() function takes this into account and bails out
> > immediately if the device already has DMA ops assigned. However, the
> > corresponding arch_teardown_dma_ops() function, called from driver
> > unbind code paths (including probe deferral), will tear the mapping down
> > regardless of who created it. When the device is reprobed
> > arch_setup_dma_ops() will be called again but won't perform any
> > operation as the DMA ops will still be set.
> > 
> > We need to reset the DMA ops in arch_teardown_dma_ops() to fix this.
> > However, we can't do so unconditionally, as then a new mapping would be
> > created by arch_setup_dma_ops() when the device is reprobed, regardless
> > of whether the device needs to share a mapping or not. We must thus keep
> > track of whether arch_setup_dma_ops() created the mapping, and only in
> > that case tear it down in arch_teardown_dma_ops().
> > 
> > Keep track of that information in the dev_archdata structure. As the
> > structure is embedded in all instances of struct device let's not grow
> > it, but turn the existing dma_coherent bool field into a bitfield that
> > can be used for other purposes.
> > 
> > Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred
> > probing or error") Signed-off-by: Laurent Pinchart
> >  ---
> > 
> >  arch/arm/include/asm/device.h | 3 ++-
> >  arch/arm/mm/dma-mapping.c | 5 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
> > index 36ec9c8f6e16..3234fe9bba6e 100644
> > --- a/arch/arm/include/asm/device.h
> > +++ b/arch/arm/include/asm/device.h
> > @@ -19,7 +19,8 @@ struct dev_archdata {
> >  #ifdef CONFIG_XEN
> > const struct dma_map_ops *dev_dma_ops;
> >  #endif
> > -   bool dma_coherent;
> > +   unsigned int dma_coherent:1;
> 
> This should only ever be accessed by the Xen DMA code via the
> is_device_dma_coherent() helper, so I can't see the change of storage
> type causing any problems.

Thank you for double-checking. I agree with your analysis.

> > +   unsigned int dma_ops_setup:1;
> >  };
> >  
> >  struct omap_device;
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index c742dfd2967b..e0272f9140e2 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -2430,9 +2430,14 @@ void arch_setup_dma_ops(struct device *dev, u64
> > dma_base, u64 size,
> > dev->dma_ops = xen_dma_ops;
> > }
> >  #endif
> > +   dev->archdata.dma_ops_setup = true;
> >  }
> >  
> >  void arch_teardown_dma_ops(struct device *dev)
> >  {
> > +   if (!dev->archdata.dma_ops_setup)
> > +   return;
> > +
> > arm_teardown_iommu_dma_ops(dev);
> > +   set_dma_ops(dev, NULL);
> 
> Should we clear dma_ops_setup here for symmetry? I guess in practice
> it's down to the IOMMU driver so will never change after the first
> probe, but it still feels like a bit of a nagging loose end.

To make a difference, we would need an IOMMU driver that creates a mapping 
after a first round of arch_setup_dma_ops() / arch_teardown_dma_ops() calls, 
follow by a second round. I don't think this could happen, but if it did, I 
believe we'd be screwed already, as there would be a time were an incorrect 
mapping (created by arch_setup_dma_ops() while the IOMMU driver needs to take 
care of mapping creation) exists.

> With that (or firm reassurance that it's OK not to),
> 
> Reviewed-by: Robin Murphy 
> 
> Apologies for being too arm64-focused in the earlier reviews and
> overlooking this. Should the patch supersede 8674/1 currently in
> Russell's incoming box?

Yes I think it should. Could you please take care of that ?

You can also add my

Tested-by: Laurent Pinchart 

as I've tested that this paptch restores proper IOMMU operation on the Renesas 
R-Car H2 Lager board. I believe the problem related to Sricharan's patch 
reported by Geert still affects us and needs to be addressed separately.

-- 
Regards,

Laurent Pinchart

[renesas-drivers:master 101/531] drivers/media//platform/vsp1/vsp1_video.c:951:30: warning: missing braces around initializer

2017-05-16 Thread kbuild test robot

tree:   
https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git master
head:   1e9588d83e43b54a15094cd0fa125f8a5645d6ea
commit: 5debeb08338b520f52577ca6cf9be815a54c07ea [101/531] v4l: vsp1: Provide a 
writeback video device
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 5debeb08338b520f52577ca6cf9be815a54c07ea
# save the attached .config to linux build tree
make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   drivers/media//platform/vsp1/vsp1_video.c: In function 
'vsp1_video_wb_process_buffer':
>> drivers/media//platform/vsp1/vsp1_video.c:951:30: warning: missing braces 
>> around initializer [-Wmissing-braces]
  video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
 ^
   drivers/media//platform/vsp1/vsp1_video.c:951:30: warning: (near 
initialization for '(anonymous).addr') [-Wmissing-braces]
   drivers/media//platform/vsp1/vsp1_video.c: In function 
'vsp1_video_wb_stop_streaming':
   drivers/media//platform/vsp1/vsp1_video.c:1013:22: warning: missing braces 
around initializer [-Wmissing-braces]
 rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
 ^
   drivers/media//platform/vsp1/vsp1_video.c:1013:22: warning: (near 
initialization for '(anonymous).addr') [-Wmissing-braces]

vim +951 drivers/media//platform/vsp1/vsp1_video.c

   935  }
   936  
   937  buf = list_first_entry_or_null(&video->wbqueue, struct 
vsp1_vb2_buffer,
   938  queue);
   939  
   940  if (buf) {
   941  video->rwpf->mem = buf->mem;
   942  
   943  /*
   944   * Store this buffer as pending. It will commence at 
the next
   945   * frame start interrupt
   946   */
   947  video->pending = buf;
   948  list_del(&buf->queue);
   949  } else {
   950  /* Disable writeback with no buffer */
 > 951  video->rwpf->mem = (struct vsp1_rwpf_memory) { 0 };
   952  }
   953  
   954  spin_unlock_irqrestore(&video->irqlock, flags);
   955  }
   956  
   957  static void vsp1_video_wb_frame_end(struct vsp1_pipeline *pipe)
   958  {
   959  struct vsp1_video *video = pipe->output->video;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [RFC PATCH v2 1/4] media: i2c: adv748x: add adv748x driver

2017-05-16 Thread Sakari Ailus

Hi Kieran,

On Tue, May 16, 2017 at 01:56:05PM +0100, Kieran Bingham wrote:
...
>  +static int adv748x_afe_g_input_status(struct v4l2_subdev *sd, u32 
>  *status)
>  +{
>  +struct adv748x_state *state = adv748x_afe_to_state(sd);
>  +int ret;
>  +
>  +ret = mutex_lock_interruptible(&state->mutex);
> >>>
> >>> I wonder if this is necessary. Do you expect the mutex to be taken for
> >>> extended periods of time?
> >>
> >> It looks like the only other adv* driver to take a lock here is the 7180.
> >> Perhaps that is where the heritage of this code derived from.
> >>
> >> I don't think the locking should be held for a long time anywhere, but I 
> >> will
> >> try to go through and consider all the locking throughout the code base.
> >>
> >> At the moment I think anything that calls adv748x_afe_status() should be 
> >> locked
> >> to ensure serialisation through adv748x_afe_read_ro_map().
> > 
> > I meant whether you need the "_interruptible" part. It's quite a bit of
> > repeating error handling that looks mostly unnecessary.
> > 
> 
> Aha - I see what you mean now...
> 
> Most of these use I2C transactions though, so that would be our source of any 
> delay.
> 
> If it's acceptable to expect an I2C bus to never hang, or if the I2C subsystem
> can handle that, then we can chop the interruptible ... but I'm not sure I2C 
> bus
> transactions are guaranteed like that ? Don't things like clock stretching, 
> and
> unreliable hardware leave us vulnerable there...

$ git grep mutex_lock_interruptible -- drivers/media/i2c/
drivers/media/i2c/adv7180.c:int err = 
mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:int ret = 
mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:int ret = 
mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:int ret = 
mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:ret = mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:int ret = 
mutex_lock_interruptible(&state->mutex);
drivers/media/i2c/adv7180.c:ret = mutex_lock_interruptible(&state->mutex);

That's all.

...

>  +int adv748x_afe_probe(struct adv748x_state *state, struct device_node 
>  *ep)
>  +{
>  +int ret;
>  +unsigned int i;
>  +
>  +state->afe.streaming = false;
>  +state->afe.curr_norm = V4L2_STD_ALL;
>  +
>  +adv748x_subdev_init(&state->afe.sd, state, &adv748x_afe_ops, 
>  "afe");
>  +
>  +/* Ensure that matching is based upon the endpoint fwnodes */
>  +state->afe.sd.fwnode = &ep->fwnode;
> >>>
> >>> I wonder if you really need to convert all users of async matching to use
> >>> endpoints --- could you opportunistically peek if the device node matches,
> >>> just in case? You can't have an accidental positive match anyway.
> >>>
> >>> So, the match is now for plain fwnode pointers, and it would be:
> >>>
> >>> return async->fwnode == fwnode ||
> >>>   port_parent(parent(async->fwnode)) == fwnode ||
> >>>   async->fwnode == port_parent(parent(fwnode));
> >>
> >>
> >> If we adapt the core to match against endpoint comparisons and device node
> >> comparisons then yes we wouldn't have to adapt everything, I.e. we 
> >> wouldn't have
> >> to change all the async devices - but we would have to adapt all the 
> >> bus/bridge
> >> drivers (those who perform the v4l2_async_notifier_register() call) as 
> >> they must
> >> register their notifier against an endpoint if they are to ever be able to
> >> connect to an ADV748x or other device which will rely upon multiple 
> >> endpoints.
> > 
> > If there really is a need to, we can add a new framework helper function for
> > that --- as I think Niklas proposed. I'm not really sure it should be really
> > needed though; e.g. the omap3isp driver does without that.
> > 
> 
> I'm afraid I don't understand what you mean by omap3isp doing without ...

Sorry. I think I lost you somewhere --- I meant matching in the few drivers
that compare fwnodes. That'd break as a result unless the matching was
updated accordingly.

It'd be nice to remove that matching as well though but I think it'd be
safer not to.

> 
> In omap3isp/isp.c: isp_fwnodes_parse() the async notifier is registered 
> looking
> at the endpoints parent fwnode:
> 
>   isd->asd.match.fwnode.fwnode =
>   fwnode_graph_get_remote_port_parent(fwnode);
> 
> This would therefore not support ADV748x ... (it wouldn't know which TX/CSI
> entity to match against)
> 
> So the way I see it is that all devices which currently register an async
> notifier should now register the match against the endpoint instead of the
> parent device:
> 
>  - isd->asd.match.fwnode.fwnode = fwnode_graph_get_remote_port_parent(fwnode);
>  + isd->asd.match.fwnode.fwnode = fwnode;
> 
> And then if we adapt the match to check against:
>fwnode == fwnode |

[PATCH v2 0/5] R-Car DU: Fix IOMMU operation when connected to VSP

2017-05-16 Thread Laurent Pinchart

Hello,

This patch series fixes the rcar-du-drm driver to support VSP plane sources
with an IOMMU. It is available for convenience at

git://linuxtv.org/pinchartl/media.git iommu/devel/du

On R-Car Gen3 the DU has no direct memory access but sources planes through
VSP instances. When an IOMMU is inserted between the VSP and memory, the DU
framebuffers need to be DMA mapped using the VSP device, not the DU device as
currently done. The same situation can also be reproduced on Gen2 hardware by
linking the VSP to the DU in DT [1], effectively disabling direct memory
access by the DU.

The situation is made quite complex by the fact that different planes can be
connected to different DU instances, and thus served by different IOMMUs (or,
in practice on existing hardware, by the same IOMMU but through different
micro-TLBs). We thus can't allocate and map buffers to the right device in a
single dma_alloc_wc() operation as done in the DRM CMA GEM helpers.

However, on such setups, the DU DT node doesn't reference IOMMUs as the DU
does not perform any direct memory access. We can thus keep the GEM object
allocation unchanged, and the DMA addresses that we receive in the DU driver
will be physical addresses. Those buffers then need to be mapped to the VSP
device when they are associated with planes. Fortunately the atomic framework
provides two plane helper operations, .prepare_fb() and .cleanup_fb() that we
can use for this purpose.

The reality is slightly more complex than this on Gen3, as an FCP device
instance sits between VSP instances and memory. It is the FCP devices that are
connected to the IOMMUs, and buffer mapping thus need to be performed using
the FCP devices. This isn't required on Gen2 as the platforms don't have any
FCPs.

Patches 1/5 and 2/5 extend the rcar-fcp driver API to expose the FCP struct
device. Patch 3/5 then updates the vsp1 driver to map the display lists and
video buffers through the FCP when it exists. This alone fixes VSP operation
with an IOMMU on R-Car Gen3 systems.

Moving on to addressing the DU issue, patch 4/5 extends the vsp1 driver API to
allow mapping a scatter-gather list to the VSP, with the implementation using
the FCP devices instead when available. Patch 5/5 finally uses the vsp1
mapping API in the rcar-du-drm driver to map and unmap buffers when needed.

The series has been tested on the H2 Lager board and M3-W Salvator-X boards.
The IOMMU is known not to work properly on the H3 ES1.1, so the H3 Salvator-X
board hasn't been tested. In all cases both the DU and VSP operation has been
tested, and tests were run with and without linking the DU and VSP devices to
the IOMMU in DT.

For H2, the patches were tested on top of v4.12-rc1 with a set of out-of-tree
patches to link the VSP and DU to the IOMMUs and to enable VSP+DU combined
similar to R-Car Gen3, and an additional DMA mapping API patch [2] that fixes
IOMMU operation on ARM32, currently broken in v4.12-rc1. For M3-W, they were
were tested on top of renesas-drivers-2017-05-16-v4.12-rc1 with a set of
out-of-tree patches to add FCP, VSP, DU and IPMMU instances to the M3-W DT, as
well as a hack for the IPMMU driver to whitelist all bus master devices.

All tests passed successfully. The issue previously noticed on H3 with
synchronization between page flip and VSP operation that was caused by buffers
getting unmapped (and possibly freed) before the VSP was done reading them is
now gone thanks to the VSP+DU flicker fix that should be merged in v4.13 and
is available in renesas-drivers-2017-05-16-v4.12-rc1.

A possible improvement is to modify the GEM object allocation mechanism to use
non-contiguous memory when the DU driver detects that all the VSP instances it
is connected to use an IOMMU (possibly through FCP devices).

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg06589.html
[2] https://www.spinics.net/lists/arm-kernel/msg581410.html

Laurent Pinchart (4):
  v4l: rcar-fcp: Don't get/put module reference
  v4l: rcar-fcp: Add an API to retrieve the FCP device
  v4l: vsp1: Add API to map and unmap DRM buffers through the VSP
  drm: rcar-du: Map memory through the VSP device

Magnus Damm (1):
  v4l: vsp1: Map the DL and video buffers through the proper bus master

 drivers/gpu/drm/rcar-du/rcar_du_vsp.c| 74 +---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h|  2 +
 drivers/media/platform/rcar-fcp.c| 17 
 drivers/media/platform/vsp1/vsp1.h   |  1 +
 drivers/media/platform/vsp1/vsp1_dl.c|  4 +-
 drivers/media/platform/vsp1/vsp1_drm.c   | 25 +++
 drivers/media/platform/vsp1/vsp1_drv.c   |  9 
 drivers/media/platform/vsp1/vsp1_video.c |  2 +-
 include/media/rcar-fcp.h |  5 +++
 include/media/vsp1.h |  3 ++
 10 files changed, 124 insertions(+), 18 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v2 1/5] v4l: rcar-fcp: Don't get/put module reference

2017-05-16 Thread Laurent Pinchart

Direct callers of the FCP API hold a reference to the FCP module due to
module linkage, there's no need to take another one manually. Take a
reference to the device instead to ensure that it won't disappear behind
the caller's back.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/rcar-fcp.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/rcar-fcp.c 
b/drivers/media/platform/rcar-fcp.c
index 7146fc5ef168..e9f609edf513 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -53,14 +53,7 @@ struct rcar_fcp_device *rcar_fcp_get(const struct 
device_node *np)
if (fcp->dev->of_node != np)
continue;
 
-   /*
-* Make sure the module won't be unloaded behind our back. This
-* is a poor man's safety net, the module should really not be
-* unloaded while FCP users can be active.
-*/
-   if (!try_module_get(fcp->dev->driver->owner))
-   fcp = NULL;
-
+   get_device(fcp->dev);
goto done;
}
 
@@ -81,7 +74,7 @@ EXPORT_SYMBOL_GPL(rcar_fcp_get);
 void rcar_fcp_put(struct rcar_fcp_device *fcp)
 {
if (fcp)
-   module_put(fcp->dev->driver->owner);
+   put_device(fcp->dev);
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
-- 
Regards,

Laurent Pinchart

[PATCH v2 5/5] drm: rcar-du: Map memory through the VSP device

2017-05-16 Thread Laurent Pinchart

For planes handled by a VSP instance, map the framebuffer memory through
the VSP to ensure proper IOMMU handling.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 74 ---
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h |  2 +
 2 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index b0ff304ce3dc..1b874cfd40f3 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -19,7 +19,9 @@
 #include 
 #include 
 
+#include 
 #include 
+#include 
 #include 
 
 #include 
@@ -170,12 +172,9 @@ static void rcar_du_vsp_plane_setup(struct 
rcar_du_vsp_plane *plane)
cfg.dst.width = state->state.crtc_w;
cfg.dst.height = state->state.crtc_h;
 
-   for (i = 0; i < state->format->planes; ++i) {
-   struct drm_gem_cma_object *gem;
-
-   gem = drm_fb_cma_get_gem_obj(fb, i);
-   cfg.mem[i] = gem->paddr + fb->offsets[i];
-   }
+   for (i = 0; i < state->format->planes; ++i)
+   cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
+  + fb->offsets[i];
 
for (i = 0; i < ARRAY_SIZE(formats_kms); ++i) {
if (formats_kms[i] == state->format->fourcc) {
@@ -187,6 +186,67 @@ static void rcar_du_vsp_plane_setup(struct 
rcar_du_vsp_plane *plane)
vsp1_du_atomic_update(plane->vsp->vsp, plane->index, &cfg);
 }
 
+static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane,
+   struct drm_plane_state *state)
+{
+   struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+   struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+   struct rcar_du_device *rcdu = vsp->dev;
+   unsigned int i;
+   int ret;
+
+   if (!state->fb)
+   return 0;
+
+   for (i = 0; i < rstate->format->planes; ++i) {
+   struct drm_gem_cma_object *gem =
+   drm_fb_cma_get_gem_obj(state->fb, i);
+   struct sg_table *sgt = &rstate->sg_tables[i];
+
+   ret = dma_get_sgtable(rcdu->dev, sgt, gem->vaddr, gem->paddr,
+ gem->base.size);
+   if (ret)
+   goto fail;
+
+   ret = vsp1_du_map_sg(vsp->vsp, sgt);
+   if (!ret) {
+   sg_free_table(sgt);
+   ret = -ENOMEM;
+   goto fail;
+   }
+   }
+
+   return 0;
+
+fail:
+   for (i--; i >= 0; i--) {
+   struct sg_table *sgt = &rstate->sg_tables[i];
+
+   vsp1_du_unmap_sg(vsp->vsp, sgt);
+   sg_free_table(sgt);
+   }
+
+   return ret;
+}
+
+static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane,
+struct drm_plane_state *state)
+{
+   struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state);
+   struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp;
+   unsigned int i;
+
+   if (!state->fb)
+   return;
+
+   for (i = 0; i < rstate->format->planes; ++i) {
+   struct sg_table *sgt = &rstate->sg_tables[i];
+
+   vsp1_du_unmap_sg(vsp->vsp, sgt);
+   sg_free_table(sgt);
+   }
+}
+
 static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane,
  struct drm_plane_state *state)
 {
@@ -227,6 +287,8 @@ static void rcar_du_vsp_plane_atomic_update(struct 
drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs rcar_du_vsp_plane_helper_funcs = {
+   .prepare_fb = rcar_du_vsp_plane_prepare_fb,
+   .cleanup_fb = rcar_du_vsp_plane_cleanup_fb,
.atomic_check = rcar_du_vsp_plane_atomic_check,
.atomic_update = rcar_du_vsp_plane_atomic_update,
 };
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index f1d0f1824528..8861661590ff 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -43,6 +43,7 @@ static inline struct rcar_du_vsp_plane 
*to_rcar_vsp_plane(struct drm_plane *p)
  * struct rcar_du_vsp_plane_state - Driver-specific plane state
  * @state: base DRM plane state
  * @format: information about the pixel format used by the plane
+ * @sg_tables: scatter-gather tables for the frame buffer memory
  * @alpha: value of the plane alpha property
  * @zpos: value of the plane zpos property
  */
@@ -50,6 +51,7 @@ struct rcar_du_vsp_plane_state {
struct drm_plane_state state;
 
const struct rcar_du_format_info *format;
+   struct sg_table sg_tables[3];
 
unsigned int alpha;
unsigned int zpos;
-- 
Regards,

Laurent Pinchart

[PATCH v2 4/5] v4l: vsp1: Add API to map and unmap DRM buffers through the VSP

2017-05-16 Thread Laurent Pinchart

The display buffers must be mapped for DMA through the device that
performs memory access. Expose an API to map and unmap memory through
the VSP device to be used by the DU.

As all the buffers allocated by the DU driver are coherent, we can skip
cache handling when mapping and unmapping them. This will need to be
revisited when support for non-coherent buffers will be added to the DU
driver.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 25 +
 include/media/vsp1.h   |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index 9d235e830f5a..c809c2aadca0 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -12,9 +12,11 @@
  */
 
 #include 
+#include 
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
@@ -524,6 +526,29 @@ void vsp1_du_atomic_flush(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_flush);
 
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt)
+{
+   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+   /*
+* As all the buffers allocated by the DU driver are coherent, we can
+* skip cache sync. This will need to be revisited when support for
+* non-coherent buffers will be added to the DU driver.
+*/
+   return dma_map_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
+   DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_map_sg);
+
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt)
+{
+   struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+   dma_unmap_sg_attrs(vsp1->bus_master, sgt->sgl, sgt->nents,
+  DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
+
 /* 
-
  * Initialization
  */
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 38aac554dbba..6aa630c9f7af 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -13,6 +13,7 @@
 #ifndef __MEDIA_VSP1_H__
 #define __MEDIA_VSP1_H__
 
+#include 
 #include 
 #include 
 
@@ -46,5 +47,7 @@ void vsp1_du_atomic_begin(struct device *dev);
 int vsp1_du_atomic_update(struct device *dev, unsigned int rpf,
  const struct vsp1_du_atomic_config *cfg);
 void vsp1_du_atomic_flush(struct device *dev);
+int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
+void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
 
 #endif /* __MEDIA_VSP1_H__ */
-- 
Regards,

Laurent Pinchart

[PATCH v2 3/5] v4l: vsp1: Map the DL and video buffers through the proper bus master

2017-05-16 Thread Laurent Pinchart

From: Magnus Damm 

On Gen2 hardware the VSP1 is a bus master and accesses the display list
and video buffers through DMA directly. On Gen3 hardware, however,
memory accesses go through a separate IP core called FCP.

The VSP1 driver unconditionally maps DMA buffers through the VSP device.
While this doesn't cause any practical issue so far, DMA mappings will
be incorrect as soon as we will enable IOMMU support for the FCP on Gen3
platforms, resulting in IOMMU faults.

Fix this by mapping all buffers through the FCP device if present, and
through the VSP1 device as usual otherwise.

Suggested-by: Magnus Damm 
[Cache the bus master device]
Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/vsp1/vsp1.h   | 1 +
 drivers/media/platform/vsp1/vsp1_dl.c| 4 ++--
 drivers/media/platform/vsp1/vsp1_drv.c   | 9 +
 drivers/media/platform/vsp1/vsp1_video.c | 2 +-
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1.h 
b/drivers/media/platform/vsp1/vsp1.h
index 85387a64179a..847963b6e9eb 100644
--- a/drivers/media/platform/vsp1/vsp1.h
+++ b/drivers/media/platform/vsp1/vsp1.h
@@ -74,6 +74,7 @@ struct vsp1_device {
 
void __iomem *mmio;
struct rcar_fcp_device *fcp;
+   struct device *bus_master;
 
struct vsp1_bru *bru;
struct vsp1_clu *clu;
diff --git a/drivers/media/platform/vsp1/vsp1_dl.c 
b/drivers/media/platform/vsp1/vsp1_dl.c
index 7d8f37772b56..445d1c31fff3 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -137,7 +137,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
dlb->vsp1 = vsp1;
dlb->size = size;
 
-   dlb->entries = dma_alloc_wc(vsp1->dev, dlb->size, &dlb->dma,
+   dlb->entries = dma_alloc_wc(vsp1->bus_master, dlb->size, &dlb->dma,
GFP_KERNEL);
if (!dlb->entries)
return -ENOMEM;
@@ -150,7 +150,7 @@ static int vsp1_dl_body_init(struct vsp1_device *vsp1,
  */
 static void vsp1_dl_body_cleanup(struct vsp1_dl_body *dlb)
 {
-   dma_free_wc(dlb->vsp1->dev, dlb->size, dlb->entries, dlb->dma);
+   dma_free_wc(dlb->vsp1->bus_master, dlb->size, dlb->entries, dlb->dma);
 }
 
 /**
diff --git a/drivers/media/platform/vsp1/vsp1_drv.c 
b/drivers/media/platform/vsp1/vsp1_drv.c
index 048446af5ae7..95c26edead85 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -764,6 +764,15 @@ static int vsp1_probe(struct platform_device *pdev)
PTR_ERR(vsp1->fcp));
return PTR_ERR(vsp1->fcp);
}
+
+   /*
+* When the FCP is present, it handles all bus master accesses
+* for the VSP and must thus be used in place of the VSP device
+* to map DMA buffers.
+*/
+   vsp1->bus_master = rcar_fcp_get_device(vsp1->fcp);
+   } else {
+   vsp1->bus_master = vsp1->dev;
}
 
/* Configure device parameters based on the version register. */
diff --git a/drivers/media/platform/vsp1/vsp1_video.c 
b/drivers/media/platform/vsp1/vsp1_video.c
index eab3c3ea85d7..5af3486afe07 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -1197,7 +1197,7 @@ struct vsp1_video *vsp1_video_create(struct vsp1_device 
*vsp1,
video->queue.ops = &vsp1_video_queue_qops;
video->queue.mem_ops = &vb2_dma_contig_memops;
video->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-   video->queue.dev = video->vsp1->dev;
+   video->queue.dev = video->vsp1->bus_master;
ret = vb2_queue_init(&video->queue);
if (ret < 0) {
dev_err(video->vsp1->dev, "failed to initialize vb2 queue\n");
-- 
Regards,

Laurent Pinchart

[PATCH v2 2/5] v4l: rcar-fcp: Add an API to retrieve the FCP device

2017-05-16 Thread Laurent Pinchart

The new rcar_fcp_get_device() function retrieves the struct device
related to the FCP device. This is useful to handle DMA mapping through
the right device.

Signed-off-by: Laurent Pinchart 
---
 drivers/media/platform/rcar-fcp.c | 6 ++
 include/media/rcar-fcp.h  | 5 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/media/platform/rcar-fcp.c 
b/drivers/media/platform/rcar-fcp.c
index e9f609edf513..2988031d285d 100644
--- a/drivers/media/platform/rcar-fcp.c
+++ b/drivers/media/platform/rcar-fcp.c
@@ -78,6 +78,12 @@ void rcar_fcp_put(struct rcar_fcp_device *fcp)
 }
 EXPORT_SYMBOL_GPL(rcar_fcp_put);
 
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+   return fcp->dev;
+}
+EXPORT_SYMBOL_GPL(rcar_fcp_get_device);
+
 /**
  * rcar_fcp_enable - Enable an FCP
  * @fcp: The FCP instance
diff --git a/include/media/rcar-fcp.h b/include/media/rcar-fcp.h
index 8723f05c6321..b60a7b176c37 100644
--- a/include/media/rcar-fcp.h
+++ b/include/media/rcar-fcp.h
@@ -19,6 +19,7 @@ struct rcar_fcp_device;
 #if IS_ENABLED(CONFIG_VIDEO_RENESAS_FCP)
 struct rcar_fcp_device *rcar_fcp_get(const struct device_node *np);
 void rcar_fcp_put(struct rcar_fcp_device *fcp);
+struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp);
 int rcar_fcp_enable(struct rcar_fcp_device *fcp);
 void rcar_fcp_disable(struct rcar_fcp_device *fcp);
 #else
@@ -27,6 +28,10 @@ static inline struct rcar_fcp_device *rcar_fcp_get(const 
struct device_node *np)
return ERR_PTR(-ENOENT);
 }
 static inline void rcar_fcp_put(struct rcar_fcp_device *fcp) { }
+static inline struct device *rcar_fcp_get_device(struct rcar_fcp_device *fcp)
+{
+   return NULL;
+}
 static inline int rcar_fcp_enable(struct rcar_fcp_device *fcp)
 {
return 0;
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] pinctrl: sh-pfc: r8a7796: SSI_{WS/SCK}34 -> SSI_{WS/SCK}349

2017-05-16 Thread Kuninori Morimoto


Hi Geert

> >> From: Kuninori Morimoto 
> >>
> >> R-Car Gen3 is using SSI_{WS/SCK}349 instead of SSI_{WS/SCK}34.
> >> But, current code is based on old datasheet which had typo.
> >> This patch fixes this typo.
> >>
> >> Signed-off-by: Kuninori Morimoto 
> >> ---
> >> v1 -> v2
> >
> > Reviewed-by: Geert Uytterhoeven 
> > i.e. will queue in sh-pfc-for-v4.13.
> 
> As I haven't send a pull request yet, I'm moving the definition part of the
> rename up, and folded the ssi349_ctrl part into "pinctrl: sh-pfc: r8a7796: Add
> Audio clock pin support".

OK, thanks


Best regards
---
Kuninori Morimoto

Re: ASoC: rsnd: fix sound route path when using SRC6/SRC9

2017-05-16 Thread Kuninori Morimoto


Hi Geert

> > @@ -70,16 +71,19 @@ static int rsnd_cmd_init(struct rsnd_mod *mod,
> > } else {
> > struct rsnd_mod *src = rsnd_io_to_mod_src(io);
> >
> > -   u32 path[] = {
> > -   [0] = 0x3,
> > -   [1] = 0x30001,
> > -   [2] = 0x4,
> > -   [3] = 0x1,
> > -   [4] = 0x2,
> > -   [5] = 0x40100
> > +   u8 cmd_case[] = {
> > +   [0] = 0x3,
> > +   [1] = 0x3,
> > +   [2] = 0x4,
> > +   [3] = 0x1,
> > +   [4] = 0x2,
> > +   [5] = 0x4,
> > +   [6] = 0x1,
> > +   [9] = 0x2,
> > };
> >
> > -   data = path[rsnd_mod_id(src)];
> > +   data = path[rsnd_mod_id(src)] |
> 
> With gcc 4.9.0:
> 
> sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init':
> sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below
> array bounds [-Warray-bounds]
>data = path[rsnd_mod_id(src)] |

Hmm
I tried random code/compile, but gcc behavior seems strange.
Fore example if I removed next line, this warning disappeard

-   data = path[rsnd_mod_id(src)] |
-   cmd_case[rsnd_mod_id(src)] << 16;
+   data = path[rsnd_mod_id(src)];

Is this related to this ?
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59124



Best regards
---
Kuninori Morimoto

[renesas:topic/sdhi-refactor-v2+sdhi-cmd23 6/7] ERROR: "renesas_sdhi_remove" [drivers/mmc/host/renesas_sdhi_sys_dmac.ko] undefined!

2017-05-16 Thread kbuild test robot

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git 
topic/sdhi-refactor-v2+sdhi-cmd23
head:   799c8fd6d9386e2ea6880a89c9de3fbb7a7994f7
commit: 2156a07053f52c85fba8b715116b0241d5fca25b [6/7] mmc: renesas-sdhi: make 
renesas_sdhi_sys_dmac main module file
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 2156a07053f52c85fba8b715116b0241d5fca25b
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "renesas_sdhi_remove" [drivers/mmc/host/renesas_sdhi_sys_dmac.ko] 
>> undefined!
>> ERROR: "renesas_sdhi_probe" [drivers/mmc/host/renesas_sdhi_sys_dmac.ko] 
>> undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v5 3/3] phy: Group vendor specific phy drivers

2017-05-16 Thread Vivek Gautam




On 05/16/2017 03:40 PM, Kishon Vijay Abraham I wrote:

Hi Vivek,

On Thursday 11 May 2017 12:17 PM, Vivek Gautam wrote:

Adding vendor specific directories in phy to group
phy drivers under their respective vendor umbrella.

Also updated the MAINTAINERS file to reflect the correct
directory structure for phy drivers.

Signed-off-by: Vivek Gautam 
Acked-by: Heiko Stuebner 
Acked-by: Viresh Kumar 
Acked-by: Krzysztof Kozlowski 
Acked-by: Yoshihiro Shimoda 
Reviewed-by: Jaehoon Chung 
Cc: Kishon Vijay Abraham I 
Cc: David S. Miller 
Cc: Geert Uytterhoeven 
Cc: Yoshihiro Shimoda 
Cc: Guenter Roeck 
Cc: Heiko Stuebner 
Cc: Viresh Kumar 
Cc: Maxime Ripard 
Cc: Chen-Yu Tsai 
Cc: Sylwester Nawrocki 
Cc: Krzysztof Kozlowski 
Cc: Jaehoon Chung 
Cc: Stephen Boyd 
Cc: Martin Blumenstingl 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-o...@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-...@vger.kernel.org
---

Changes since v4:
  - Reprepared the patch based on latest torvalds/master.
  - Added new directory for amlogic, since there's a new patch [1]
coming in for amlogic platforms.

Changes since v3:
  - Added 'Acked-by' and 'Reviewed by' tags received for this patch.
  - No functional change.

Changes since v2:
  - Rebased on linux-phy/next.
  - Took care of drivers added/removed for each vendors since v2.
  - Updated the 'Signed-off-by' tag with my current email id.

Changes from v1:
  - Updated the MAINTAINERS file to reflect the same change
in directory structure.
  - Removed PHY_PLAT config as pointed out by Kishon.
So we don't require the second patch in the v1 of this series:
[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency
  - Renamed sunxi --> allwinner; rcar --> renesas.
  - Fixed error coming with qcom Makefile.

* Build tested multi_v7_defconfig.
* Build tested arm64 defconfig with all the involved configs enabled.

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


.
.

.
.

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index afaf7b643eeb..b353ac603ea0 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,73 +15,6 @@ config GENERIC_PHY
  phy users can obtain reference to the PHY. All the users of this

.
.

.
.

-
+menu "Platform Phy drivers"

I don't think creating a new menu is required. I removed it and merged into
linux-phy.


Right. No need for a new menu. Thank you for fixing it and pushing out.

Best Regards
Vivek



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


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

Re: [PATCH] ARM: dma-mapping: Don't tear third-party mappings

2017-05-16 Thread Sricharan R

Hi Laurent/Robin,

On 5/16/2017 10:14 PM, Laurent Pinchart wrote:
> Hi Robin,
> 
> On Tuesday 16 May 2017 16:47:36 Robin Murphy wrote:
>> On 16/05/17 16:14, Laurent Pinchart wrote:
>>> arch_setup_dma_ops() is used in device probe code paths to create an
>>> IOMMU mapping and attach it to the device. The function assumes that the
>>> device is attached to a device-specific IOMMU instance (or at least a
>>> device-specific TLB in a shared IOMMU instance) and thus creates a
>>> separate mapping for every device.
>>>
>>> On several systems (Renesas R-Car Gen2 being one of them), that
>>> assumption is not true, and IOMMU mappings must be shared between
>>> multiple devices. In those cases the IOMMU driver knows better than the
>>> generic ARM dma-mapping layer and attaches mapping to devices manually
>>> with arm_iommu_attach_device(), which sets the DMA ops for the device.
>>>
>>> The arch_setup_dma_ops() function takes this into account and bails out
>>> immediately if the device already has DMA ops assigned. However, the
>>> corresponding arch_teardown_dma_ops() function, called from driver
>>> unbind code paths (including probe deferral), will tear the mapping down
>>> regardless of who created it. When the device is reprobed
>>> arch_setup_dma_ops() will be called again but won't perform any
>>> operation as the DMA ops will still be set.
>>>
>>> We need to reset the DMA ops in arch_teardown_dma_ops() to fix this.
>>> However, we can't do so unconditionally, as then a new mapping would be
>>> created by arch_setup_dma_ops() when the device is reprobed, regardless
>>> of whether the device needs to share a mapping or not. We must thus keep
>>> track of whether arch_setup_dma_ops() created the mapping, and only in
>>> that case tear it down in arch_teardown_dma_ops().
>>>
>>> Keep track of that information in the dev_archdata structure. As the
>>> structure is embedded in all instances of struct device let's not grow
>>> it, but turn the existing dma_coherent bool field into a bitfield that
>>> can be used for other purposes.
>>>
>>> Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred
>>> probing or error") Signed-off-by: Laurent Pinchart
>>>  ---
>>>
>>>  arch/arm/include/asm/device.h | 3 ++-
>>>  arch/arm/mm/dma-mapping.c | 5 +
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
>>> index 36ec9c8f6e16..3234fe9bba6e 100644
>>> --- a/arch/arm/include/asm/device.h
>>> +++ b/arch/arm/include/asm/device.h
>>> @@ -19,7 +19,8 @@ struct dev_archdata {
>>>  #ifdef CONFIG_XEN
>>> const struct dma_map_ops *dev_dma_ops;
>>>  #endif
>>> -   bool dma_coherent;
>>> +   unsigned int dma_coherent:1;
>>
>> This should only ever be accessed by the Xen DMA code via the
>> is_device_dma_coherent() helper, so I can't see the change of storage
>> type causing any problems.
> 
> Thank you for double-checking. I agree with your analysis.
> 
>>> +   unsigned int dma_ops_setup:1;
>>>  };
>>>  
>>>  struct omap_device;
>>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>>> index c742dfd2967b..e0272f9140e2 100644
>>> --- a/arch/arm/mm/dma-mapping.c
>>> +++ b/arch/arm/mm/dma-mapping.c
>>> @@ -2430,9 +2430,14 @@ void arch_setup_dma_ops(struct device *dev, u64
>>> dma_base, u64 size,
>>> dev->dma_ops = xen_dma_ops;
>>> }
>>>  #endif
>>> +   dev->archdata.dma_ops_setup = true;
>>>  }
>>>  
>>>  void arch_teardown_dma_ops(struct device *dev)
>>>  {
>>> +   if (!dev->archdata.dma_ops_setup)
>>> +   return;
>>> +
>>> arm_teardown_iommu_dma_ops(dev);
>>> +   set_dma_ops(dev, NULL);
>>
>> Should we clear dma_ops_setup here for symmetry? I guess in practice
>> it's down to the IOMMU driver so will never change after the first
>> probe, but it still feels like a bit of a nagging loose end.
> 
> To make a difference, we would need an IOMMU driver that creates a mapping 
> after a first round of arch_setup_dma_ops() / arch_teardown_dma_ops() calls, 
> follow by a second round. I don't think this could happen, but if it did, I 
> believe we'd be screwed already, as there would be a time were an incorrect 
> mapping (created by arch_setup_dma_ops() while the IOMMU driver needs to take 
> care of mapping creation) exists.
> 

Feels correct not to reset this, the iommu drivers in question, seems to
creating mapping/attaching in add_device path (which gets called before the
clients gets probed) and when the iommu client gets deferred/reprobed that
does not happen again even after the first round.

>> With that (or firm reassurance that it's OK not to),
>>
>> Reviewed-by: Robin Murphy 
>>
>> Apologies for being too arm64-focused in the earlier reviews and
>> overlooking this. Should the patch supersede 8674/1 currently in
>> Russell's incoming box?
> 
> Yes I think it should. Could you please take care of that ?
> 
> You can also add my
> was
> Tested-by: Laurent Pinchart 
> 
> as I've te

[PATCH] ASoC: rsnd: check src mod pointer for rsnd_mod_id()

2017-05-16 Thread Kuninori Morimoto


From: Kuninori Morimoto 

Without this patch, gcc 4.9.x says

sound/soc/sh/rcar/cmd.c: In function 'rsnd_cmd_init':
sound/soc/sh/rcar/cmd.c:85:14: warning: array subscript is below array\
bounds [-Warray-bounds]
data = path[rsnd_mod_id(src)] |

Signed-off-by: Kuninori Morimoto 
---
 sound/soc/sh/rcar/cmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/sh/rcar/cmd.c b/sound/soc/sh/rcar/cmd.c
index d879c01..9a136d8 100644
--- a/sound/soc/sh/rcar/cmd.c
+++ b/sound/soc/sh/rcar/cmd.c
@@ -82,6 +82,9 @@ static int rsnd_cmd_init(struct rsnd_mod *mod,
[9] = 0x2,
};
 
+   if (unlikely(!src))
+   return -EIO;
+
data = path[rsnd_mod_id(src)] |
cmd_case[rsnd_mod_id(src)] << 16;
}
-- 
1.9.1

60 matches

Mail list logo