[PATCH v2 5/6] usb: gadget: udc: renesas_usb3: should fail if devm_phy_get() returns error

[Yoshihiro Shimoda]

This patch fixes an issue that this driver ignores errors other than
the non-existence of the device, f.e. a memory allocation failure
in devm_phy_get(). So, this patch replaces devm_phy_get() with
devm_phy_optional_get().

Reported-by: Simon Horman 
Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic 
phy")
Cc:  # v4.15+
Signed-off-by: Yoshihiro Shimoda 
---
Remarks:
 - A new file in v2

 drivers/usb/gadget/udc/renesas_usb3.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 233bc04..0e70163 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2636,9 +2636,11 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
 * This is optional. So, if this driver cannot get a phy,
 * this driver will not handle a phy anymore.
 */
-   usb3->phy = devm_phy_get(>dev, "usb");
-   if (IS_ERR(usb3->phy))
-   usb3->phy = NULL;
+   usb3->phy = devm_phy_optional_get(>dev, "usb");
+   if (IS_ERR(usb3->phy)) {
+   ret = PTR_ERR(usb3->phy);
+   goto err_add_udc;
+   }
 
pm_runtime_enable(>dev);
ret = usb_add_gadget_udc(>dev, >gadget);
-- 
1.9.1

[PATCH v2 4/6] usb: gadget: udc: renesas_usb3: should call devm_phy_get() before add udc

[Yoshihiro Shimoda]

This patch fixes an issue that this driver cannot call phy_init()
if a gadget driver is alreadly loaded because usb_add_gadget_udc()
might call renesas_usb3_start() via .udc_start.
This patch also revises the typo (s/an optional/optional/).

Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic 
phy")
Cc:  # v4.15+
Signed-off-by: Yoshihiro Shimoda 
Reviewed-by: Simon Horman 
---
Changes from v1:
 - Add Reviewed-by.
 - Revise typo in the comment.

 drivers/usb/gadget/udc/renesas_usb3.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 738b734..233bc04 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2632,6 +2632,14 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
if (ret < 0)
goto err_alloc_prd;
 
+   /*
+* This is optional. So, if this driver cannot get a phy,
+* this driver will not handle a phy anymore.
+*/
+   usb3->phy = devm_phy_get(>dev, "usb");
+   if (IS_ERR(usb3->phy))
+   usb3->phy = NULL;
+
pm_runtime_enable(>dev);
ret = usb_add_gadget_udc(>dev, >gadget);
if (ret < 0)
@@ -2641,14 +2649,6 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
if (ret < 0)
goto err_dev_create;
 
-   /*
-* This is an optional. So, if this driver cannot get a phy,
-* this driver will not handle a phy anymore.
-*/
-   usb3->phy = devm_phy_get(>dev, "usb");
-   if (IS_ERR(usb3->phy))
-   usb3->phy = NULL;
-
usb3->workaround_for_vbus = priv->workaround_for_vbus;
 
renesas_usb3_debugfs_init(usb3, >dev);
-- 
1.9.1

[PATCH v2 1/6] usb: gadget: udc: renesas_usb3: fix double phy_put()

[Yoshihiro Shimoda]

This patch fixes an issue that this driver cause double phy_put()
calling. This driver must not call phy_put() in the remove because
the driver calls devm_phy_get() in the probe.

Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic 
phy")
Cc:  # v4.15+
Signed-off-by: Yoshihiro Shimoda 
Reviewed-by: Simon Horman 
---
Changes from v1:
 - Add Reviewed-by.

 drivers/usb/gadget/udc/renesas_usb3.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 409cde4..78a12a5 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2408,8 +2408,6 @@ static int renesas_usb3_remove(struct platform_device 
*pdev)
renesas_usb3_dma_free_prd(usb3, >dev);
 
__renesas_usb3_ep_free_request(usb3->ep0_req);
-   if (usb3->phy)
-   phy_put(usb3->phy);
pm_runtime_disable(>dev);
 
return 0;
-- 
1.9.1

[PATCH v2 6/6] usb: gadget: udc: renesas_usb3: disable the controller's irqs for reconnecting

[Yoshihiro Shimoda]

This patch fixes an issue that reconnection is possible to fail
because unexpected state handling happens by the irqs. To fix the issue,
the driver disables the controller's irqs when disconnected.

Fixes: 746bfe63bba3 ("usb: gadget: renesas_usb3: add support for Renesas USB3.0 
peripheral controller")
Cc:  # v4.5+
Signed-off-by: Yoshihiro Shimoda 
---

Remarks:
 - A new file in v2

 drivers/usb/gadget/udc/renesas_usb3.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 0e70163..5caf78b 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -623,6 +623,13 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
usb3_usb2_pullup(usb3, 0);
usb3_clear_bit(usb3, USB30_CON_B3_CONNECT, USB3_USB30_CON);
usb3_reset_epc(usb3);
+   usb3_disable_irq_1(usb3, USB_INT_1_B2_RSUM | USB_INT_1_B3_PLLWKUP |
+  USB_INT_1_B3_LUPSUCS | USB_INT_1_B3_DISABLE |
+  USB_INT_1_SPEED | USB_INT_1_B3_WRMRST |
+  USB_INT_1_B3_HOTRST | USB_INT_1_B2_SPND |
+  USB_INT_1_B2_L1SPND | USB_INT_1_B2_USBRST);
+   usb3_clear_bit(usb3, USB_COM_CON_SPD_MODE, USB3_USB_COM_CON);
+   usb3_init_epc_registers(usb3);
 
if (usb3->driver)
usb3->driver->disconnect(>gadget);
-- 
1.9.1

[PATCH v2 3/6] usb: gadget: udc: renesas_usb3: should call pm_runtime_enable() before add udc

[Yoshihiro Shimoda]

This patch fixes an issue that this driver causes panic if a gadget
driver is already loaded because usb_add_gadget_udc() might call
renesas_usb3_start() via .udc_start, and then pm_runtime_get_sync()
in renesas_usb3_start() doesn't work correctly.
Note that the usb3_to_dev() macro should not be called at this timing
because the macro uses the gadget structure.

Fixes: cf06df3fae28 ("usb: gadget: udc: renesas_usb3: move 
pm_runtime_{en,dis}able()")
Cc:  # v4.15+
Signed-off-by: Yoshihiro Shimoda 
Reviewed-by: Simon Horman 
---
Changes from v1:
 - Add Reviewed-by.

 drivers/usb/gadget/udc/renesas_usb3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 1c54a77..738b734 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -2632,6 +2632,7 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
if (ret < 0)
goto err_alloc_prd;
 
+   pm_runtime_enable(>dev);
ret = usb_add_gadget_udc(>dev, >gadget);
if (ret < 0)
goto err_add_udc;
@@ -2653,7 +2654,6 @@ static int renesas_usb3_probe(struct platform_device 
*pdev)
renesas_usb3_debugfs_init(usb3, >dev);
 
dev_info(>dev, "probed%s\n", usb3->phy ? " with phy" : "");
-   pm_runtime_enable(usb3_to_dev(usb3));
 
return 0;
 
-- 
1.9.1

[PATCH v2 0/6] usb: gadget: udc: renesas_usb3: fix some major issues

[Yoshihiro Shimoda]

This patch set is based on v4.16.

Changes from v1:
 - Add Reviewed-by in patch 1, 2, 3 and 4.
 - Revise typo in patch 4.
 - Add new patches as patch 5 and 6.

Yoshihiro Shimoda (6):
  usb: gadget: udc: renesas_usb3: fix double phy_put()
  usb: gadget: udc: renesas_usb3: should remove debugfs
  usb: gadget: udc: renesas_usb3: should call pm_runtime_enable() before
add udc
  usb: gadget: udc: renesas_usb3: should call devm_phy_get() before add
udc
  usb: gadget: udc: renesas_usb3: should fail if devm_phy_get() returns
error
  usb: gadget: udc: renesas_usb3: disable the controller's irqs for
reconnecting

 drivers/usb/gadget/udc/renesas_usb3.c | 37 +++
 1 file changed, 25 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH v2 2/6] usb: gadget: udc: renesas_usb3: should remove debugfs

[Yoshihiro Shimoda]

This patch fixes an issue that this driver doesn't remove its debugfs.

Fixes: 43ba968b00ea ("usb: gadget: udc: renesas_usb3: add debugfs to set the 
b-device mode")
Cc:  # v4.14+
Signed-off-by: Yoshihiro Shimoda 
Reviewed-by: Simon Horman 
---
Changes from v1:
 - Add Reviewed-by.

 drivers/usb/gadget/udc/renesas_usb3.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
b/drivers/usb/gadget/udc/renesas_usb3.c
index 78a12a5..1c54a77 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -333,6 +333,7 @@ struct renesas_usb3 {
struct extcon_dev *extcon;
struct work_struct extcon_work;
struct phy *phy;
+   struct dentry *dentry;
 
struct renesas_usb3_ep *usb3_ep;
int num_usb3_eps;
@@ -2393,8 +2394,12 @@ static void renesas_usb3_debugfs_init(struct 
renesas_usb3 *usb3,
 
file = debugfs_create_file("b_device", 0644, root, usb3,
   _usb3_b_device_fops);
-   if (!file)
+   if (!file) {
dev_info(dev, "%s: Can't create debugfs mode\n", __func__);
+   debugfs_remove_recursive(root);
+   } else {
+   usb3->dentry = root;
+   }
 }
 
 /*--- platform_driver */
@@ -2402,6 +2407,7 @@ static int renesas_usb3_remove(struct platform_device 
*pdev)
 {
struct renesas_usb3 *usb3 = platform_get_drvdata(pdev);
 
+   debugfs_remove_recursive(usb3->dentry);
device_remove_file(>dev, _attr_role);
 
usb_del_gadget_udc(>gadget);
-- 
1.9.1

RE: [PATCH 4/4] usb: gadget: udc: renesas_usb3: should call devm_phy_get() before add udc

[Yoshihiro Shimoda]

Hi Simon-san,

> From: Simon Horman, Sent: Monday, April 9, 2018 8:58 PM
> 
> On Mon, Apr 02, 2018 at 09:21:34PM +0900, Yoshihiro Shimoda wrote:
> > This patch fixes an issue that this driver cannot call phy_init()
> > if a gadget driver is alreadly loaded because usb_add_gadget_udc()
> > might call renesas_usb3_start() via .udc_start.
> >
> > Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for 
> > generic phy")
> > Cc:  # v4.15+
> > Signed-off-by: Yoshihiro Shimoda 
> > ---
> >  drivers/usb/gadget/udc/renesas_usb3.c | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> Reviewed-by: Simon Horman 
> 
> 
> Please see some suggestions for follow-up lower-priority patches below.

Thank you for the review and suggestions!

> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 738b734..671bac3 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -2632,6 +2632,14 @@ static int renesas_usb3_probe(struct platform_device 
> > *pdev)
> > if (ret < 0)
> > goto err_alloc_prd;
> >
> > +   /*
> > +* This is an optional. So, if this driver cannot get a phy,
> > +* this driver will not handle a phy anymore.
> > +*/
> 
> nit: s/an optional/optional/

Oops. I will fix and submit v2 patches.

> > +   usb3->phy = devm_phy_get(>dev, "usb");
> > +   if (IS_ERR(usb3->phy))
> > +   usb3->phy = NULL;
> 
> I think this will unintentionally ignore errors other than the
> non-existence of the device, f.e. a memory allocation failure
> in devm_phy_get().
> 
> So perhaps something like this, as per phy_optional_get(), would be better?
> 
>   usb3->phy = devm_phy_get(>dev, "usb");
>   if (IS_ERR(usb3->phy) && (PTR_ERR(usb3->phy) == -ENODEV))
>   usb3->phy = NULL;

Thank you for the suggestion. I agree with you.
I think I should use devm_phy_optional_get() instead of dev_phy_get(), as you 
mentioned :)
So, I will fix the code by using devm_phy_optional_get() first, and then fix 
this.

Best regards,
Yoshihiro Shimoda

> > +
> > pm_runtime_enable(>dev);
> > ret = usb_add_gadget_udc(>dev, >gadget);
> > if (ret < 0)
> > @@ -2641,14 +2649,6 @@ static int renesas_usb3_probe(struct platform_device 
> > *pdev)
> > if (ret < 0)
> > goto err_dev_create;
> >
> > -   /*
> > -* This is an optional. So, if this driver cannot get a phy,
> > -* this driver will not handle a phy anymore.
> > -*/
> > -   usb3->phy = devm_phy_get(>dev, "usb");
> > -   if (IS_ERR(usb3->phy))
> > -   usb3->phy = NULL;
> > -
> > usb3->workaround_for_vbus = priv->workaround_for_vbus;
> >
> > renesas_usb3_debugfs_init(usb3, >dev);
> > --
> > 1.9.1
> >

Re: [PATCH] mtd: partitions: Handle add_mtd_device() failures gracefully

[Marek Vasut]

On 04/09/2018 02:25 PM, Geert Uytterhoeven wrote:
> Currently add_mtd_device() failures are plainly ignored, which may lead
> to kernel crashes later.
> 
> E.g. after flipping SW17 on r8a7791/koelsch, to switch from the large to
> the small QSPI FLASH, without updating the partition description in DT,
> the following happens:
> 
> m25p80 spi0.0: found s25sl032p, expected s25fl512s
> 3 fixed-partitions partitions found on MTD device spi0.0
> Creating 3 MTD partitions on "spi0.0":
> 0x-0x0008 : "loader"
> 0x0008-0x0060 : "user"
> mtd: partition "user" extends beyond the end of device "spi0.0" -- size 
> truncated to 0x38
> 
> The second partition is truncated correctly.
> 
> 0x0060-0x0400 : "flash"
> mtd: partition "flash" is out of reach -- disabled
> 
> The third partition is disabled by allocate_partition(), which means
> fields like erasesize are not filled in.  Hence add_mtd_device() fails
> and screams, rightfully:
> 
> [ cut here ]
> WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:508 
> add_mtd_device+0x2a0/0x2e0
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
> 4.16.0-koelsch-08649-g58e35e77b00c075d #4029
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0x7c/0x9c)
> [] (dump_stack) from [] (__warn+0xd4/0x104)
> [] (__warn) from [] (warn_slowpath_null+0x38/0x44)
> [] (warn_slowpath_null) from [] 
> (add_mtd_device+0x2a0/0x2e0)
> [] (add_mtd_device) from [] 
> (add_mtd_partitions+0xd0/0x16c)
> [] (add_mtd_partitions) from [] 
> (mtd_device_parse_register+0xc4/0x1b4)
> [] (mtd_device_parse_register) from [] 
> (m25p_probe+0x148/0x188)
> [] (m25p_probe) from [] (spi_drv_probe+0x84/0xa0)
> 
> [...]
> 
> ---[ end trace d43ce221bca7ab5c ]---
> 
> However, that failure is ignored by add_mtd_partitions(), leading to a
> crash later:
> 
> [ cut here ]
> kernel BUG at fs/sysfs/file.c:330!
> Internal error: Oops - BUG: 0 [#1] SMP ARM
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW
> 4.16.0-koelsch-08649-g58e35e77b00c075d #4029
> Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
> PC is at sysfs_create_file_ns+0x24/0x40
> LR is at 0x1
> pc : []lr : [<0001>]psr: 6013
> sp : eb447c00  ip :   fp : c0e20174
> r10: 0003  r9 : c0e20150  r8 : eb7e3818
> r7 : ea8b20f8  r6 : c0e2017c  r5 :   r4 : 
> r3 : 0200  r2 :   r1 : c0e2019c  r0 : 
> Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 30c5387d  Table: 40003000  DAC: 
> Process swapper/0 (pid: 1, stack limit = 0x7eba272f)
> Stack: (0xeb447c00 to 0xeb448000)
> 
> [...]
> 
> [] (sysfs_create_file_ns) from [] 
> (sysfs_create_files+0x34/0x70)
> [] (sysfs_create_files) from [] 
> (mtd_add_partition_attrs+0x10/0x34)
> [] (mtd_add_partition_attrs) from [] 
> (add_mtd_partitions+0xd8/0x16c)
> [] (add_mtd_partitions) from [] 
> (mtd_device_parse_register+0xc4/0x1b4)
> [] (mtd_device_parse_register) from [] 
> (m25p_probe+0x148/0x188)
> [] (m25p_probe) from [] (spi_drv_probe+0x84/0xa0)
> 
> Fix this by ignoring and freeing partitions that failed to add in
> add_mtd_partitions().  The same issue is present in mtd_add_partition(),
> so fix that as well.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
> I don't know if it is worthwhile factoring out the common handling.
> 
> Should allocate_partition() fail instead?  There's a comment saying
> "let's register it anyway to preserve ordering".
> ---
>  drivers/mtd/mtdpart.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
> index 023516a632766c42..d41adc1397dcf95e 100644
> --- a/drivers/mtd/mtdpart.c
> +++ b/drivers/mtd/mtdpart.c
> @@ -637,7 +637,14 @@ int mtd_add_partition(struct mtd_info *parent, const 
> char *name,
>   list_add(>list, _partitions);
>   mutex_unlock(_partitions_mutex);
>  
> - add_mtd_device(>mtd);
> + ret = add_mtd_device(>mtd);
> + if (ret) {
> + mutex_lock(_partitions_mutex);
> + list_del(>list);
> + mutex_unlock(_partitions_mutex);
> + free_partition(new);
> + return ret;
> + }
>  
>   mtd_add_partition_attrs(new);
>  
> @@ -731,7 +738,7 @@ int add_mtd_partitions(struct mtd_info *master,
>  {
>   struct mtd_part *slave;
>   uint64_t cur_offset = 0;
> - int i;
> + int i, ret;
>  
>   printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, 
> master->name);
>  
> @@ -746,7 +753,15 @@ int add_mtd_partitions(struct mtd_info 

Re: [PATCH/RFT v3 2/3] dt-bindings: thermal: rcar-thermal: add R8A77995 support

[Rob Herring]

On Tue, Apr 03, 2018 at 09:43:04PM +0900, Yoshihiro Kaneko wrote:
> Signed-off-by: Yoshihiro Kaneko 
> Reviewed-by: Geert Uytterhoeven 
> ---
>  Documentation/devicetree/bindings/thermal/rcar-thermal.txt | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt 
> b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> index 349e635..5ab5fcd 100644
> --- a/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/rcar-thermal.txt
> @@ -3,7 +3,8 @@
>  Required properties:
>  - compatible : "renesas,thermal-",
>  "renesas,rcar-gen2-thermal" (with thermal-zone) or
> -"renesas,rcar-thermal" (without thermal-zone) as 
> fallback.
> +"renesas,rcar-thermal" (without thermal-zone) as
> +   fallback except R-Car D3.
> Examples with soctypes are:
>   - "renesas,thermal-r8a73a4" (R-Mobile APE6)
>   - "renesas,thermal-r8a7743" (RZ/G1M)
> @@ -12,13 +13,15 @@ Required properties:
>   - "renesas,thermal-r8a7791" (R-Car M2-W)
>   - "renesas,thermal-r8a7792" (R-Car V2H)
>   - "renesas,thermal-r8a7793" (R-Car M2-N)
> + - "renesas,thermal-r8a77995" (R-Car D3)
>  - reg: Address range of the thermal registers.
> The 1st reg will be recognized as common register
> if it has "interrupts".
>  
>  Option properties:
>  
> -- interrupts : use interrupt
> +- interrupts : use interrupt.
> +  Should contain 3 interrupts for R-Car D3.

And how many for other chips?

>  
>  Example (non interrupt support):
>  
> -- 
> 1.9.1
> 

Re: [PATCH v3 2/8] DT: reset: renesas,rzn1-reboot: document RZ/N1 reboot driver

[Rob Herring]

On Thu, Mar 29, 2018 at 08:46:58AM +0100, Michel Pollet wrote:
> The Renesas RZ/N1 Family (Part #R9A06G0xx) requires a driver
> as part of the sysctrl MFD to handle rebooting the CA7 cores.
> This documents the driver bindings.
> 
> Signed-off-by: Michel Pollet 
> ---
>  .../bindings/power/renesas,rzn1-reboot.txt   | 20 
> 
>  1 file changed, 20 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/power/renesas,rzn1-reboot.txt
> 
> diff --git a/Documentation/devicetree/bindings/power/renesas,rzn1-reboot.txt 
> b/Documentation/devicetree/bindings/power/renesas,rzn1-reboot.txt
> new file mode 100644
> index 000..f592769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/renesas,rzn1-reboot.txt
> @@ -0,0 +1,20 @@
> +DT bindings for the Renesas RZ/N1 Reboot Driver
> +
> +== Reboot Driver Node ==
> +
> +The reboot driver is always a subnode of the system controller node, see
> +renesas,rzn1-sysctrl.txt for details.
> +
> +Bindings:
> ++ Required:
> + compatible = "renesas,rzn1-reboot";
> +
> +Example:
> + sysctrl: sysctrl@4000c000 {
> + compatible = "renesas,rzn1-sysctrl", "syscon", "simple-mfd";

Are there other functions for this block? If so, please define a 
complete binding for the block.

> + reg = <0x4000c000 0x1000>;
> +
> + reboot {
> + compatible = "renesas,rzn1-reboot";

Why is this node needed? The driver for "renesas,rzn1-sysctrl" should 
be able to register as a reboot handler/driver/provider.

Rob

Re: [PATCH] base: dma-mapping: Postpone cpu addr translation on mmap()

[Christoph Hellwig]

On Mon, Apr 09, 2018 at 06:59:08PM +0200, Jacopo Mondi wrote:
> I'm still a bit puzzled on what happens if dma_mmap_from_dev_coherent() fails.
> Does a dma_mmap_from_dev_coherent() failure guarantee anyhow that the
> successive virt_to_page() isn't problematic as it is today?
> Or is it the
>   if (off < count && user_count <= (count - off))
> check that makes the translation safe?

It doesn't.  I think one major issue is that we should not simply fall
to dma_common_mmap if no method is required, but need every instance of
dma_map_ops to explicitly opt into an mmap method that is known to work.

>  #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
>   unsigned long user_count = vma_pages(vma);
>   unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
>   unsigned long off = vma->vm_pgoff;
> + unsigned long pfn;
> 
>   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> 
> @@ -235,6 +235,7 @@ int dma_common_mmap(struct device *dev, struct 
> vm_area_struct *vma,
>   return ret;
> 
>   if (off < count && user_count <= (count - off)) {
> + pfn = page_to_pfn(virt_to_page(cpu_addr));
>   ret = remap_pfn_range(vma, vma->vm_start,
> pfn + off,
> user_count << PAGE_SHIFT,

Why not:

ret = remap_pfn_range(vma, vma->vm_start,
page_to_pfn(virt_to_page(cpu_addr)) + off,

and save the temp variable?

[PATCH] base: dma-mapping: Postpone cpu addr translation on mmap()

[Jacopo Mondi]

Postpone calling virt_to_page() translation on memory locations not
guaranteed to be backed by a struct page.

This patch fixes a specific issue of SH architecture configured with
SPARSEMEM memory model, when mapping buffers allocated with the memblock
APIs at system initialization time, and thus not backed by the page
infrastructure.

It does apply to the general case though, as an early translation is anyhow
incorrect and shall be postponed after trying to map memory from the device
coherent memory pool first.

Suggested-by: Laurent Pinchart 
Signed-off-by: Jacopo Mondi 

---
Compared to the RFC version I have tried to generalize the commit message,
please suggest any improvement to that.

I'm still a bit puzzled on what happens if dma_mmap_from_dev_coherent() fails.
Does a dma_mmap_from_dev_coherent() failure guarantee anyhow that the
successive virt_to_page() isn't problematic as it is today?
Or is it the
if (off < count && user_count <= (count - off))
check that makes the translation safe?

Thanks
   j

---
 drivers/base/dma-mapping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index 3b11835..8b4ec34 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -226,8 +226,8 @@ int dma_common_mmap(struct device *dev, struct 
vm_area_struct *vma,
 #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
unsigned long user_count = vma_pages(vma);
unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-   unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
unsigned long off = vma->vm_pgoff;
+   unsigned long pfn;

vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

@@ -235,6 +235,7 @@ int dma_common_mmap(struct device *dev, struct 
vm_area_struct *vma,
return ret;

if (off < count && user_count <= (count - off)) {
+   pfn = page_to_pfn(virt_to_page(cpu_addr));
ret = remap_pfn_range(vma, vma->vm_start,
  pfn + off,
  user_count << PAGE_SHIFT,
--
2.7.4

Re: [PATCH] phy: Renesas R-Car gen3 PCIe PHY driver

[Geert Uytterhoeven]

Hi Sergei,

On Mon, Apr 9, 2018 at 5:57 PM, Sergei Shtylyov
 wrote:
> On 04/09/2018 12:46 PM, Geert Uytterhoeven wrote:
>>> +static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
>>> +{
>>> +   struct device *dev = >dev;
>>> +   struct phy_provider *provider;
>>> +   struct rcar_gen3_phy *phy;
>>> +   struct resource *res;
>>> +   void __iomem *base;
>>> +   int error;
>>> +
>>> +   if (!dev->of_node) {
>>> +   dev_err(dev,
>>> +   "This driver must only be instantiated from the 
>>> device tree\n");
>>> +   return -EINVAL;
>>> +   }
>>
>> Do we need this check? Just go bang, like most other DT-only drivers do?
>
>You mean NULL pointer dereference?

Yep. (S)he who instantiates a "phy_rcar_gen3_pcie" device without DT is
bound to die.

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] phy: Renesas R-Car gen3 PCIe PHY driver

[Sergei Shtylyov]

On 04/09/2018 12:46 PM, Geert Uytterhoeven wrote:

>> This PHY is still mostly undocumented -- the only documented registers
>> exist on R-Car V3H (R8A77980) SoC  where this PHY stays in a powered-down
>> state after a reset and thus  we must power it on for PCIe to work...
> 
> Bogus spaces slipping in again?

   Yes, I should have worked in the type-setting. :-)

>> Signed-off-by: Sergei Shtylyov 
> 
>> --- /dev/null
>> +++ linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
>> @@ -0,0 +1,32 @@
[...]
>> +Example (R-Car V3H):
>> +
>> +   pcie-phy@e65d {
>> +   compatible = "renesas,r8a77980-pcie-phy",
>> +"renesas,rcar-gen3-pcie-phy";
> 
> Is the R8A77980 PCIe PHY compatible with the generic version, given it needs
> to be powered up explicitly?

   I assumed it's upwardly compatible. However, given the lack of information,
it might not be...

> The only documented register is the lane reversal register, do we need 
> bindings
> to configure it?

   Don't know, I'm not PCIe expert...

>> --- /dev/null
>> +++ linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
>> @@ -0,0 +1,158 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Renesas R-Car Gen2 PHY driver
> 
> Gen3 PCIe PHY

  Yeah, my overlook. Thanks for noticing...

>> +static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
>> +{
>> +   struct device *dev = >dev;
>> +   struct phy_provider *provider;
>> +   struct rcar_gen3_phy *phy;
>> +   struct resource *res;
>> +   void __iomem *base;
>> +   int error;
>> +
>> +   if (!dev->of_node) {
>> +   dev_err(dev,
>> +   "This driver must only be instantiated from the 
>> device tree\n");
>> +   return -EINVAL;
>> +   }
> 
> Do we need this check? Just go bang, like most other DT-only drivers do?

   You mean NULL pointer dereference?

> Gr{oetje,eeting}s,
> 
> Geert
> 

MBR, Sergei

Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()

[Sergei Shtylyov]

On 04/09/2018 01:54 PM, Simon Horman wrote:

>> In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
>> for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
>> the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?

   No, it's removed in the next patch.

[...]
>> Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
>> special PHY driver on the R-Car V3H the PCIEC initialization just freezes
>> the kernel --  adding the PHYRDY polling allows the init code to exit
>> gracefully on timeout (PHY starts powered down after reset on this SoC).
> 
> How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.

   Tested on R8A7791/Porter (unfortunately, I have no spare PCIe card) and
R8A77980/Condor (tried couple PCIe cards).

>>
>> Signed-off-by: Sergei Shtylyov 
>>
>> ---
>>  drivers/pci/host/pcie-rcar.c |   20 
>>  1 file changed, 20 insertions(+)
>>
>> Index: pci/drivers/pci/host/pcie-rcar.c
>> ===
>> --- pci.orig/drivers/pci/host/pcie-rcar.c
>> +++ pci/drivers/pci/host/pcie-rcar.c
>> @@ -36,6 +36,8 @@
>>  #define PCIECDR 0x20
>>  #define PCIEMSR 0x28
>>  #define PCIEINTXR   0x000400
>> +#define PCIEPHYSR   0x0007f0
>> +#define  PHYRDY 1
> 
> Can we start using the BIT() macro in this driver?

   Done by Marek for the other regs... not sure whose patch would hit the kernel
first tho...

[...]

MBR, Sergei

Re: [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()

[Sergei Shtylyov]

On 04/09/2018 01:56 PM, Simon Horman wrote:

>> Now that we've added PCIEPHYSR.PHYRDY polling to rcar_pcie_hw_init(),
>> there is no need anymore  for polling the PHY specific register in
>> rcar_pcie_hw_init_h1() -- remove it.
>>
>> Signed-off-by: Sergei Shtylyov 
> 
> This looks good, but has it been tested?

   Unfortunately, no. Yet it fully matches the R-Car H1 manual v1.00.

MBR, Sergei

Re: [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call

[Sergei Shtylyov]

On 04/09/2018 02:04 PM, Simon Horman wrote:

>> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
>> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
>> makes  sense to move that call into the driver's probe() method and then
>> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
>> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
> 
> I'm not sure the churn is worth it, but if you do then that is find by me.

   s/find/fine/? :-)
   I think it's worth it -- makes the code follow more closely the manuals
where the only gen1/2/3 specific init is PHY related.

>> Signed-off-by: Sergei Shtylyov 
>>
>> ---
>>  drivers/pci/host/pcie-rcar.c |   42 
>> ++
>>  1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> Index: pci/drivers/pci/host/pcie-rcar.c
>> ===
>> --- pci.orig/drivers/pci/host/pcie-rcar.c
>> +++ pci/drivers/pci/host/pcie-rcar.c
[...]
>> @@ -1082,17 +1078,18 @@ static int rcar_pcie_parse_map_dma_range
>>  }
>>  
>>  static const struct of_device_id rcar_pcie_of_match[] = {
>> -{ .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
>> +{ .compatible = "renesas,pcie-r8a7779",
>> +  .data = rcar_pcie_phy_init_h1 },
>>  { .compatible = "renesas,pcie-r8a7790",
>> -  .data = rcar_pcie_hw_init_gen2 },
>> +  .data = rcar_pcie_phy_init_gen2 },
>>  { .compatible = "renesas,pcie-r8a7791",
>> -  .data = rcar_pcie_hw_init_gen2 },
>> +  .data = rcar_pcie_phy_init_gen2 },
>>  { .compatible = "renesas,pcie-rcar-gen2",
>> -  .data = rcar_pcie_hw_init_gen2 },
>> +  .data = rcar_pcie_phy_init_gen2 },
>>  { .compatible = "renesas,pcie-r8a7795",
>> -  .data = rcar_pcie_hw_init_gen3 },
>> +  .data = rcar_pcie_phy_init_gen3 },
>>  { .compatible = "renesas,pcie-rcar-gen3",
>> -  .data = rcar_pcie_hw_init_gen3 },
>> +  .data = rcar_pcie_phy_init_gen3 },
>>  {},
> 
> I would avoid the line wrapping here, but its up to you.

   I didn't want to break the 80-colums limit; and then again, wanted to keep 
the initializers alike... 

>> @@ -1140,7 +1137,7 @@ static int rcar_pcie_probe(struct platfo
>>  struct rcar_pcie *pcie;
>>  unsigned int data;
>>  int err;
>> -int (*hw_init_fn)(struct rcar_pcie *);
>> +int (*phy_init_fn)(struct rcar_pcie *);
> 
> Looking at this I wonder if we also need a phy_cleanup() code or
> similar.

   Makes sense -- iff we start supporting PM?..

[...]

MBR, Sergei

Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()

[Laurent Pinchart]

Hi Rich,

On Monday, 9 April 2018 18:11:13 EEST Rich Felker wrote:
> On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote:
> > On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote:
> >> On 09/04/18 08:25, jacopo mondi wrote:
> >>> Hi Robin, Laurent,
> >>> 
> >>>  a long time passed, sorry about this.
> >>> 
> >>> On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote:
>  On 14/11/17 17:08, Jacopo Mondi wrote:
> > On SH4 architecture, with SPARSEMEM memory model, translating page
> > to pfn hangs the CPU. Post-pone translation to pfn after
> > dma_mmap_from_dev_coherent() function call as it succeeds and make
> > page translation not necessary.
> > 
> > This patch was suggested by Laurent Pinchart and he's working to
> > submit a proper fix mainline. Not sending for inclusion at the
> > moment.
>  
>  Y'know, I think this patch does have some merit by itself - until we
>  know that cpu_addr *doesn't* represent some device-private memory
>  which is not guaranteed to be backed by a struct page, calling
>  virt_to_page() on it is arguably semantically incorrect, even if it
>  might happen to be benign in most cases.
> >>> 
> >>> I still need to carry this patch in my trees to have a working dma
> >>> memory mapping on SH4 platforms. My understanding from your comment is
> >>> that there may be a way forward for this patch, do you still think the
> >>> same? Have you got any suggestion on how to improve this eventually
> >>> for inclusion?
> >> 
> >> As before, the change itself does seem reasonable; it might be worth
> >> rewording the commit message in more general terms rather than making it
> >> sound like an SH-specific workaround (which I really don't think it is),
> >> but otherwise I'd say just repost it as a non-RFC patch.
> > 
> > I actually can't remember any better fix I would have in mind, so this
> > looks good to me :-) I agree with Robin, the commit message should be
> > reworded. Robin's explanation of why virt_to_page() should be postponed
> > until we know that cpu_addr represents memory that is guaranteed to be
> > backed by a struct page is a good starting point. You can mention SH4 as
> > an example of an architecture that will crash when calling virt_to_page()
> > in such a case, but the fix isn't specific to SH4.
> 
> Just checking since I joined late -- is the consensus that this does
> not require any action/review specific to SH (in my role as maintainer
> way behind on maintenance)?

I don't think it requires any action specific to SH. If you want to review the 
patch in the context of SH though, please feel free to do so :-)

-- 
Regards,

Laurent Pinchart

Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()

[Rich Felker]

On Mon, Apr 09, 2018 at 04:06:15PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote:
> > On 09/04/18 08:25, jacopo mondi wrote:
> > > Hi Robin, Laurent,
> > > 
> > >  a long time passed, sorry about this.
> > > 
> > > On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote:
> > >> On 14/11/17 17:08, Jacopo Mondi wrote:
> > >>> On SH4 architecture, with SPARSEMEM memory model, translating page to
> > >>> pfn hangs the CPU. Post-pone translation to pfn after
> > >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page
> > >>> translation not necessary.
> > >>> 
> > >>> This patch was suggested by Laurent Pinchart and he's working to submit
> > >>> a proper fix mainline. Not sending for inclusion at the moment.
> > >> 
> > >> Y'know, I think this patch does have some merit by itself - until we know
> > >> that cpu_addr *doesn't* represent some device-private memory which is not
> > >> guaranteed to be backed by a struct page, calling virt_to_page() on it is
> > >> arguably semantically incorrect, even if it might happen to be benign in
> > >> most cases.
> > > 
> > > I still need to carry this patch in my trees to have a working dma memory
> > > mapping on SH4 platforms. My understanding from your comment is that
> > > there may be a way forward for this patch, do you still think the same?
> > > Have you got any suggestion on how to improve this eventually for
> > > inclusion?
> > 
> > As before, the change itself does seem reasonable; it might be worth
> > rewording the commit message in more general terms rather than making it
> > sound like an SH-specific workaround (which I really don't think it is),
> > but otherwise I'd say just repost it as a non-RFC patch.
> 
> I actually can't remember any better fix I would have in mind, so this looks 
> good to me :-) I agree with Robin, the commit message should be reworded. 
> Robin's explanation of why virt_to_page() should be postponed until we know 
> that cpu_addr represents memory that is guaranteed to be backed by a struct 
> page is a good starting point. You can mention SH4 as an example of an 
> architecture that will crash when calling virt_to_page() in such a case, but 
> the fix isn't specific to SH4.

Just checking since I joined late -- is the consensus that this does
not require any action/review specific to SH (in my role as maintainer
way behind on maintenance)?

Rich

Re: [PATCH/RFT v2 0/3] thermal: add support for r8a77995

[Niklas Söderlund]

Hi Simon,

On 2018-04-09 13:59:27 +0200, Simon Horman wrote:
> On Tue, Apr 03, 2018 at 08:00:50PM +0900, Yoshihiro Kaneko wrote:
> > Hi Simon-san,
> > 
> > 2018-03-30 22:49 GMT+09:00 Simon Horman :
> > > On Fri, Mar 30, 2018 at 12:13:00PM +0900, Yoshihiro Kaneko wrote:
> > >> This series adds thermal support for r8a77995.
> > >> R-Car D3 (r8a77995) have a thermal sensor module which is similar to 
> > >> Gen2.
> > >> Therefore this series adds r8a77995 support to rcar_thermal driver not
> > >> rcar_gen3_thermal driver.
> > >>
> > >> This series is based on the next branch of Zhang Rui's linux tree.
> > >
> > > I have very lightly tested this as follows after enabling
> > > CONFIG_RCAR_THERMAL in the kernel .config.
> > >
> > > # cat /sys/devices/virtual/thermal/thermal_zone0/temp
> > > 4
> > 
> > Thanks for the testing.
> > Probably 40C is reasonable, is not?
> 
> Yes, probably.
> 
> Niklas do you have any guidance here?

Yes judging from other Gen3 boards which al be it is using the gen3 
thermal driver the temperature seems to be in range of what is observed 
there in idle where the 3 different zones on those H3 for example is 
between 38C - 42C.

But I have no method of observing the real temperature other then with 
the driver that is being under test. And then tests i do is to 
increasing the CPU load in order to generate heat and observe the 
temperature increasing. But 40C seems like a reasonable value during 
idle compared to other Gen3 SoCs I looked at.

-- 
Regards,
Niklas Söderlund

Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()

[Laurent Pinchart]

Hello,

On Monday, 9 April 2018 14:11:22 EEST Robin Murphy wrote:
> On 09/04/18 08:25, jacopo mondi wrote:
> > Hi Robin, Laurent,
> > 
> >  a long time passed, sorry about this.
> > 
> > On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote:
> >> On 14/11/17 17:08, Jacopo Mondi wrote:
> >>> On SH4 architecture, with SPARSEMEM memory model, translating page to
> >>> pfn hangs the CPU. Post-pone translation to pfn after
> >>> dma_mmap_from_dev_coherent() function call as it succeeds and make page
> >>> translation not necessary.
> >>> 
> >>> This patch was suggested by Laurent Pinchart and he's working to submit
> >>> a proper fix mainline. Not sending for inclusion at the moment.
> >> 
> >> Y'know, I think this patch does have some merit by itself - until we know
> >> that cpu_addr *doesn't* represent some device-private memory which is not
> >> guaranteed to be backed by a struct page, calling virt_to_page() on it is
> >> arguably semantically incorrect, even if it might happen to be benign in
> >> most cases.
> > 
> > I still need to carry this patch in my trees to have a working dma memory
> > mapping on SH4 platforms. My understanding from your comment is that
> > there may be a way forward for this patch, do you still think the same?
> > Have you got any suggestion on how to improve this eventually for
> > inclusion?
> 
> As before, the change itself does seem reasonable; it might be worth
> rewording the commit message in more general terms rather than making it
> sound like an SH-specific workaround (which I really don't think it is),
> but otherwise I'd say just repost it as a non-RFC patch.

I actually can't remember any better fix I would have in mind, so this looks 
good to me :-) I agree with Robin, the commit message should be reworded. 
Robin's explanation of why virt_to_page() should be postponed until we know 
that cpu_addr represents memory that is guaranteed to be backed by a struct 
page is a good starting point. You can mention SH4 as an example of an 
architecture that will crash when calling virt_to_page() in such a case, but 
the fix isn't specific to SH4.

> >>> Suggested-by: Laurent Pinchart 
> >>> Signed-off-by: Jacopo Mondi 
> >>> ---
> >>> 
> >>>   drivers/base/dma-mapping.c | 3 ++-
> >>>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> >>> index e584edd..73d64d3 100644
> >>> --- a/drivers/base/dma-mapping.c
> >>> +++ b/drivers/base/dma-mapping.c
> >>> @@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct
> >>> vm_area_struct *vma,
> >>>   #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
> >>>   unsigned long user_count = vma_pages(vma);
> >>>   unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >>> - unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> >>>   unsigned long off = vma->vm_pgoff;
> >>> + unsigned long pfn;
> >>> 
> >>>   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> >>> 
> >>> @@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct
> >>> vm_area_struct *vma,
> >>>   return ret;
> >>>   
> >>>   if (off < count && user_count <= (count - off)) {
> >>> + pfn = page_to_pfn(virt_to_page(cpu_addr));
> >>>   ret = remap_pfn_range(vma, vma->vm_start,
> >>> pfn + off,
> >>> user_count << PAGE_SHIFT,

-- 
Regards,

Laurent Pinchart

Re: [PATCH] clk: renesas: r8a77980: Correct parent clock of PCIEC0

[Simon Horman]

On Mon, Apr 09, 2018 at 02:29:22PM +0200, Geert Uytterhoeven wrote:
> According to the R-Car Gen3 Hardware Manual Errata for Rev 0.80 of
> December 22, 2017, the parent clock of the PCIe module clock on R-Car
> V3H is S2D2.
> 
> Fixes: ce15783c510a9905 ("clk: renesas: cpg-mssr: add R8A77980 support")
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Simon Horman 

Re: [PATCH] ARM: debug-ll: Add support for r8a77470

[Simon Horman]

On Fri, Mar 30, 2018 at 11:28:06AM +0200, Geert Uytterhoeven wrote:
> On Thu, Mar 29, 2018 at 12:42 PM, Biju Das  wrote:
> > Enable low-level debugging support for RZ/G1C (r8a77470). RZ/G1C uses
> > SCIF1 for the debug console.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.18.

Re: [PATCH v5 0/3] R-Car DU: Fix LVDS output on Gen2 boards

[Simon Horman]

On Mon, Apr 09, 2018 at 03:16:03PM +0300, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Monday, 9 April 2018 14:17:15 EEST Simon Horman wrote:
> > On Fri, Apr 06, 2018 at 10:14:35PM +0300, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > This patch series fixes LVDS output support on the Lager, Koelsh, Porter
> > > and Gose boards that broke in v4.17-rc1 due to the combination of the
> > > R-Car DU LVDS driver rework and the DT move of all on-SoC peripherals to
> > > a /soc node.
> > > 
> > > We could handle the problem in the R-Car DU LVDS DT backward compatibility
> > > code, but that fix would only be used for v4.17 as in v4.18 the Gen2 DT
> > > will move to the new LVDS DT bindings. I thus propose merging these three
> > > patches in v4.17 already to fix the problem as this is the simplest
> > > solution.
> > > 
> > > The patches are based on top of Linus' master that includes both the R-Car
> > > DU changes and the ARM DT changes for v4.17-rc1. I can rebase them on top
> > > of v4.17-rc1 when it will be released, but I don't expect any change.
> > 
> > Thanks, I think this should be fine. Please ping me once v4.17-rc1 has
> > been released.
> 
> I'll send a v6 on top of v4.17-rc1 to fix the issue pointed out by Niklas in 
> patch 2/3.

Great, thanks!

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Simon Horman]

On Mon, Apr 09, 2018 at 01:47:38PM +0200, Geert Uytterhoeven wrote:
> Hi Simon, Marek,
> 
> On Mon, Apr 9, 2018 at 1:41 PM, Simon Horman  wrote:
> > On Mon, Apr 09, 2018 at 10:20:05AM +0200, Marek Vasut wrote:
> >> On 04/09/2018 10:07 AM, Geert Uytterhoeven wrote:
> >> > On Sun, Apr 8, 2018 at 3:09 PM, Marek Vasut  
> >> > wrote:
> >> >> From: Dien Pham 
> >> >>
> >> >> The controller clock can be switched off during suspend/resume,
> >> >> let runtime PM take care of that.
> >> >>
> >> >> Signed-off-by: Dien Pham 
> >> >> Signed-off-by: Hien Dang 
> >> >> Signed-off-by: Marek Vasut 
> >> >> Cc: Geert Uytterhoeven 
> >> >> Cc: Lorenzo Pieralisi 
> >> >> Cc: Phil Edworthy 
> >> >> Cc: Simon Horman 
> >> >> Cc: Wolfram Sang 
> >> >> Cc: linux-renesas-soc@vger.kernel.org
> >> >> To: linux-...@vger.kernel.org
> >> >> ---
> >> >> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
> >> >>   reordering of function calls in probe
> >> >> - Dispose of fail_clk in rcar_pcie_get_resources()
> >> >> V3: - Fix up the failpath in probe function
> >> >> V4: - Rebase on recent linux-next
> >> >> V5: - Do not call pci_free_resource_list(>resources) if
> >> >>   rcar_pcie_parse_request_of_pci_ranges() fails, since that
> >> >>   functiona calls pci_free_resource_list() already.
> >> >
> >> > Thanks for the update!
> >> >
> >> >> --- a/drivers/pci/host/pcie-rcar.c
> >> >> +++ b/drivers/pci/host/pcie-rcar.c
> >> >
> >> >> @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct 
> >> >> platform_device *pdev)
> >> >> if (err)
> >> >> goto err_free_bridge;
> >> >>
> >> >> +   pm_runtime_enable(pcie->dev);
> >> >> +   err = pm_runtime_get_sync(pcie->dev);
> >> >> +   if (err < 0) {
> >> >> +   dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
> >> >> +   goto err_pm_disable;
> >> >> +   }
> >> >> +
> >> >
> >> > As you moved the pm_runtime setup up...
> >> >
> >> >> err = rcar_pcie_get_resources(pcie);
> >> >> if (err < 0) {
> >> >> dev_err(dev, "failed to request resources: %d\n", err);
> >> >> -   goto err_free_resource_list;
> >> >> +   goto err_pm_put;
> >> >> }
> >> >>
> >> >> err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
> >> >> if (err)
> >> >> -   goto err_free_resource_list;
> >> >> -
> >> >> -   pm_runtime_enable(dev);
> >> >> -   err = pm_runtime_get_sync(dev);
> >> >> -   if (err < 0) {
> >> >> -   dev_err(dev, "pm_runtime_get_sync failed\n");
> >> >> -   goto err_pm_disable;
> >> >> -   }
> >> >> +   goto err_pm_put;
> >> >>
> >> >> /* Failure to get a link might just be that no cards are 
> >> >> inserted */
> >> >> hw_init_fn = of_device_get_match_data(dev);
> >> >> @@ -1174,9 +1161,8 @@ static int rcar_pcie_probe(struct platform_device 
> >> >> *pdev)
> >> >>
> >> >>  err_pm_disable:
> >> >> pm_runtime_disable(dev);
> >> >
> >> > ... shouldn't it be moved down here, for symmetry?
> >>
> >> I am reasonably certain the failpath should be correct now. Did I still
> >> miss something ?
> >
> > It looks correct to me too. Geert are Marek and I missing something?
> 
> Probably it will still work fine, but after this patch, Runtime PM is enabled
> early, and disabled early, which is not symmetrical.
> 
> I like symmetry ;-)

Understood. I think that is reasonable.
Marek, would you care to respin?

[PATCH] mtd: partitions: Handle add_mtd_device() failures gracefully

[Geert Uytterhoeven]

Currently add_mtd_device() failures are plainly ignored, which may lead
to kernel crashes later.

E.g. after flipping SW17 on r8a7791/koelsch, to switch from the large to
the small QSPI FLASH, without updating the partition description in DT,
the following happens:

m25p80 spi0.0: found s25sl032p, expected s25fl512s
3 fixed-partitions partitions found on MTD device spi0.0
Creating 3 MTD partitions on "spi0.0":
0x-0x0008 : "loader"
0x0008-0x0060 : "user"
mtd: partition "user" extends beyond the end of device "spi0.0" -- size 
truncated to 0x38

The second partition is truncated correctly.

0x0060-0x0400 : "flash"
mtd: partition "flash" is out of reach -- disabled

The third partition is disabled by allocate_partition(), which means
fields like erasesize are not filled in.  Hence add_mtd_device() fails
and screams, rightfully:

[ cut here ]
WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:508 
add_mtd_device+0x2a0/0x2e0
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
4.16.0-koelsch-08649-g58e35e77b00c075d #4029
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (dump_stack+0x7c/0x9c)
[] (dump_stack) from [] (__warn+0xd4/0x104)
[] (__warn) from [] (warn_slowpath_null+0x38/0x44)
[] (warn_slowpath_null) from [] 
(add_mtd_device+0x2a0/0x2e0)
[] (add_mtd_device) from [] 
(add_mtd_partitions+0xd0/0x16c)
[] (add_mtd_partitions) from [] 
(mtd_device_parse_register+0xc4/0x1b4)
[] (mtd_device_parse_register) from [] 
(m25p_probe+0x148/0x188)
[] (m25p_probe) from [] (spi_drv_probe+0x84/0xa0)

[...]

---[ end trace d43ce221bca7ab5c ]---

However, that failure is ignored by add_mtd_partitions(), leading to a
crash later:

[ cut here ]
kernel BUG at fs/sysfs/file.c:330!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW
4.16.0-koelsch-08649-g58e35e77b00c075d #4029
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
PC is at sysfs_create_file_ns+0x24/0x40
LR is at 0x1
pc : []lr : [<0001>]psr: 6013
sp : eb447c00  ip :   fp : c0e20174
r10: 0003  r9 : c0e20150  r8 : eb7e3818
r7 : ea8b20f8  r6 : c0e2017c  r5 :   r4 : 
r3 : 0200  r2 :   r1 : c0e2019c  r0 : 
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 30c5387d  Table: 40003000  DAC: 
Process swapper/0 (pid: 1, stack limit = 0x7eba272f)
Stack: (0xeb447c00 to 0xeb448000)

[...]

[] (sysfs_create_file_ns) from [] 
(sysfs_create_files+0x34/0x70)
[] (sysfs_create_files) from [] 
(mtd_add_partition_attrs+0x10/0x34)
[] (mtd_add_partition_attrs) from [] 
(add_mtd_partitions+0xd8/0x16c)
[] (add_mtd_partitions) from [] 
(mtd_device_parse_register+0xc4/0x1b4)
[] (mtd_device_parse_register) from [] 
(m25p_probe+0x148/0x188)
[] (m25p_probe) from [] (spi_drv_probe+0x84/0xa0)

Fix this by ignoring and freeing partitions that failed to add in
add_mtd_partitions().  The same issue is present in mtd_add_partition(),
so fix that as well.

Signed-off-by: Geert Uytterhoeven 
---
I don't know if it is worthwhile factoring out the common handling.

Should allocate_partition() fail instead?  There's a comment saying
"let's register it anyway to preserve ordering".
---
 drivers/mtd/mtdpart.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 023516a632766c42..d41adc1397dcf95e 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -637,7 +637,14 @@ int mtd_add_partition(struct mtd_info *parent, const char 
*name,
list_add(>list, _partitions);
mutex_unlock(_partitions_mutex);
 
-   add_mtd_device(>mtd);
+   ret = add_mtd_device(>mtd);
+   if (ret) {
+   mutex_lock(_partitions_mutex);
+   list_del(>list);
+   mutex_unlock(_partitions_mutex);
+   free_partition(new);
+   return ret;
+   }
 
mtd_add_partition_attrs(new);
 
@@ -731,7 +738,7 @@ int add_mtd_partitions(struct mtd_info *master,
 {
struct mtd_part *slave;
uint64_t cur_offset = 0;
-   int i;
+   int i, ret;
 
printk(KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, 
master->name);
 
@@ -746,7 +753,15 @@ int add_mtd_partitions(struct mtd_info *master,
list_add(>list, _partitions);
mutex_unlock(_partitions_mutex);
 
-   add_mtd_device(>mtd);
+   ret = add_mtd_device(>mtd);
+   if (ret) {
+   mutex_lock(_partitions_mutex);
+

Re: [PATCH] media: entity: fix spelling for media_entity_get_fwnode_pad()

[Simon Horman]

On Sun, Apr 08, 2018 at 06:11:52PM +0200, Niklas Söderlund wrote:
> From: Niklas Söderlund 
> 
> s/dose/does/
> 
> Fixes: d295c6a460cd2ac6 ("[media] media: entity: Add 
> media_entity_get_fwnode_pad() function")
> Signed-off-by: Niklas Söderlund 

Reviewed-by: Simon Horman 

[RESEND PATCH] serial: sh-sci: Document r8a77470 bindings

[Biju Das]

RZ/G1C (R8A77470) SoC also has the R-Car gen2 compatible SCIF and HSCIF
ports, so document the SoC specific bindings.

Signed-off-by: Biju Das 
Reviewed-by: Fabrizio Castro 
Reviewed-by: Geert Uytterhoeven 
Reviewed-by: Simon Horman 
---
 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt 
b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index cf504d0..1c90d2c 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -17,6 +17,8 @@ Required properties:
 - "renesas,scifa-r8a7745" for R8A7745 (RZ/G1E) SCIFA compatible UART.
 - "renesas,scifb-r8a7745" for R8A7745 (RZ/G1E) SCIFB compatible UART.
 - "renesas,hscif-r8a7745" for R8A7745 (RZ/G1E) HSCIF compatible UART.
+- "renesas,scif-r8a77470" for R8A77470 (RZ/G1C) SCIF compatible UART.
+- "renesas,hscif-r8a77470" for R8A77470 (RZ/G1C) HSCIF compatible UART.
 - "renesas,scif-r8a7778" for R8A7778 (R-Car M1) SCIF compatible UART.
 - "renesas,scif-r8a7779" for R8A7779 (R-Car H1) SCIF compatible UART.
 - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
-- 
2.7.4

Re: [PATCH 2/7] arm64: dts: renesas: r8a77970: add VSPD support

[Simon Horman]

On Fri, Apr 06, 2018 at 04:33:21PM +0300, Laurent Pinchart wrote:
> On Friday, 6 April 2018 16:08:07 EEST Jacopo Mondi wrote:
> > From: Sergei Shtylyov 
> > 
> > Describe VSPD0 in the R8A77970 device tree; it will be used by DU in
> > the next patch...
> > 
> > Based on the original (and large) patch by Daisuke Matsushita
> > .
> > 
> > Signed-off-by: Vladimir Barinov 
> > Signed-off-by: Sergei Shtylyov 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77970.dtsi | 10 ++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi
> > b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index 71f466d..db06c94 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi
> > @@ -625,6 +625,16 @@
> > power-domains = < R8A77970_PD_ALWAYS_ON>;
> > resets = < 603>;
> > };
> > +
> > +   vspd0: vsp@fea2 {
> > +   compatible = "renesas,vsp2";
> > +   reg = <0 0xfea2 0 0x4000>;
> 
> You need to extend the memory region to include the V6_CLUTn_TBL* registers. 
> I 
> would recommend simply extending it to 0x8000 as all other VSP instances, 
> even 
> if the registers at 0x7000-0x7fff are not implemented.
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 

I applied the first patch of this series. Please update this,
and any subsequent patches as appropriate and then repost the series
without the first patch.

Thanks

Re: [PATCH 1/7] arm64: dts: renesas: r8a77970: add FCPVD support

[Simon Horman]

On Fri, Apr 06, 2018 at 04:28:17PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Friday, 6 April 2018 16:08:06 EEST Jacopo Mondi wrote:
> > From: Sergei Shtylyov 
> > 
> > Describe FCPVD0 in the R8A77970 device tree; it will be used by VSPD0 in
> > the next patch...
> > 
> > Based on the original (and large) patch by Daisuke Matsushita
> > .
> > 
> > Signed-off-by: Vladimir Barinov 
> > Signed-off-by: Sergei Shtylyov 
> > Signed-off-by: Niklas Söderlund 
> 
> Reviewed-by: Laurent Pinchart 

Thanks, applied for v4.18.

Re: [RFC PATCH] mmc: renesas_sdhi_internal_dmac: use more generic whitelisting

[Simon Horman]

On Thu, Apr 05, 2018 at 06:42:16PM +0200, Wolfram Sang wrote:
> From: Wolfram Sang 
> 
> Whitelisting every ES version does not scale. So, we whitelist whole
> SoCs independent of ES version. If we need specific handling for an ES
> version, we put it to the front, so it will be matched first.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Simon Horman 

Re: [PATCH v5 0/3] R-Car DU: Fix LVDS output on Gen2 boards

[Laurent Pinchart]

Hi Simon,

On Monday, 9 April 2018 14:17:15 EEST Simon Horman wrote:
> On Fri, Apr 06, 2018 at 10:14:35PM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series fixes LVDS output support on the Lager, Koelsh, Porter
> > and Gose boards that broke in v4.17-rc1 due to the combination of the
> > R-Car DU LVDS driver rework and the DT move of all on-SoC peripherals to
> > a /soc node.
> > 
> > We could handle the problem in the R-Car DU LVDS DT backward compatibility
> > code, but that fix would only be used for v4.17 as in v4.18 the Gen2 DT
> > will move to the new LVDS DT bindings. I thus propose merging these three
> > patches in v4.17 already to fix the problem as this is the simplest
> > solution.
> > 
> > The patches are based on top of Linus' master that includes both the R-Car
> > DU changes and the ARM DT changes for v4.17-rc1. I can rebase them on top
> > of v4.17-rc1 when it will be released, but I don't expect any change.
> 
> Thanks, I think this should be fine. Please ping me once v4.17-rc1 has
> been released.

I'll send a v6 on top of v4.17-rc1 to fix the issue pointed out by Niklas in 
patch 2/3.

> > Laurent Pinchart (3):
> >   ARM: dts: r8a7790: Convert to new LVDS DT bindings
> >   ARM: dts: r8a7791: Convert to new LVDS DT bindings
> >   ARM: dts: r8a7793: Convert to new LVDS DT bindings
> >  
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 22 +---
> >  arch/arm/boot/dts/r8a7790.dtsi| 65 +-
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 10 --
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 16 +++--
> >  arch/arm/boot/dts/r8a7791.dtsi| 36 +++
> >  arch/arm/boot/dts/r8a7793-gose.dts| 10 --
> >  arch/arm/boot/dts/r8a7793.dtsi| 37 
> >  7 files changed, 163 insertions(+), 33 deletions(-)

-- 
Regards,

Laurent Pinchart

RE: [PATCH v3 1/3] serial: sh-sci: Document r8a77470 bindings

[Biju Das]

Hi Simon,

Thanks for the feedback.

> -Original Message-
> From: Simon Horman [mailto:ho...@verge.net.au]
> Sent: 09 April 2018 13:10
> To: Biju Das 
> Cc: Philipp Zabel ; Rob Herring
> ; Mark Rutland ; Magnus
> Damm ; devicet...@vger.kernel.org; linux-
> renesas-...@vger.kernel.org; Geert Uytterhoeven
> ; Chris Paterson
> ; Fabrizio Castro
> 
> Subject: Re: [PATCH v3 1/3] serial: sh-sci: Document r8a77470 bindings
>
> On Tue, Apr 03, 2018 at 12:19:37PM +0100, Biju Das wrote:
> > RZ/G1C (R8A77470) SoC also has the R-Car gen2 compatible SCIF and
> > HSCIF ports, so document the SoC specific bindings.
> >
> > Signed-off-by: Biju Das 
> > Reviewed-by: Fabrizio Castro 
> > Reviewed-by: Geert Uytterhoeven 
> > Reviewed-by: Simon Horman 
>
> I recommend reposting this as a stand-alone patch with:
>
> To: Greg Kroah-Hartman 
> Cc: Rob Herring ,
> linux-ser...@vger.kernel.org, devicet...@vger.kernel.org,
> linux-renesas-soc@vger.kernel.org

Will send standalone patch

> > ---
> > V1->V2:
> > * No Change
> > V2->V3:
> > * No Change
> >
> >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2
> > ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > index cf504d0..1c90d2c 100644
> > --- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > @@ -17,6 +17,8 @@ Required properties:
> >  - "renesas,scifa-r8a7745" for R8A7745 (RZ/G1E) SCIFA compatible UART.
> >  - "renesas,scifb-r8a7745" for R8A7745 (RZ/G1E) SCIFB compatible UART.
> >  - "renesas,hscif-r8a7745" for R8A7745 (RZ/G1E) HSCIF compatible UART.
> > +- "renesas,scif-r8a77470" for R8A77470 (RZ/G1C) SCIF compatible UART.
> > +- "renesas,hscif-r8a77470" for R8A77470 (RZ/G1C) HSCIF compatible
> UART.
> >  - "renesas,scif-r8a7778" for R8A7778 (R-Car M1) SCIF compatible UART.
> >  - "renesas,scif-r8a7779" for R8A7779 (R-Car H1) SCIF compatible UART.
> >  - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > --
> > 2.7.4
> >



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Re: [PATCH v3 1/3] serial: sh-sci: Document r8a77470 bindings

[Simon Horman]

On Tue, Apr 03, 2018 at 12:19:37PM +0100, Biju Das wrote:
> RZ/G1C (R8A77470) SoC also has the R-Car gen2 compatible SCIF and HSCIF
> ports, so document the SoC specific bindings.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 
> Reviewed-by: Simon Horman 

I recommend reposting this as a stand-alone patch with:

To: Greg Kroah-Hartman 
Cc: Rob Herring ,
linux-ser...@vger.kernel.org, devicet...@vger.kernel.org,
linux-renesas-soc@vger.kernel.org

> ---
> V1->V2:
> * No Change
> V2->V3:
> * No Change
> 
>  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt 
> b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> index cf504d0..1c90d2c 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -17,6 +17,8 @@ Required properties:
>  - "renesas,scifa-r8a7745" for R8A7745 (RZ/G1E) SCIFA compatible UART.
>  - "renesas,scifb-r8a7745" for R8A7745 (RZ/G1E) SCIFB compatible UART.
>  - "renesas,hscif-r8a7745" for R8A7745 (RZ/G1E) HSCIF compatible UART.
> +- "renesas,scif-r8a77470" for R8A77470 (RZ/G1C) SCIF compatible UART.
> +- "renesas,hscif-r8a77470" for R8A77470 (RZ/G1C) HSCIF compatible UART.
>  - "renesas,scif-r8a7778" for R8A7778 (R-Car M1) SCIF compatible UART.
>  - "renesas,scif-r8a7779" for R8A7779 (R-Car H1) SCIF compatible UART.
>  - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> -- 
> 2.7.4
> 

Re: [PATCH v3 3/3] ARM: dts: iwg23s-sbc: Add support for iWave G23S-SBC based on RZ/G1C

[Simon Horman]

On Tue, Apr 03, 2018 at 12:19:39PM +0100, Biju Das wrote:
> Add support for iWave iW-RainboW-G23S single board computer based on
>  RZ/G1C.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.18.

Re: [PATCH v3 2/3] ARM: dts: r8a77470: Initial SoC device tree

[Simon Horman]

On Tue, Apr 03, 2018 at 12:19:38PM +0100, Biju Das wrote:
> The initial R8A77470 SoC device tree including CPU0, GIC, timer, SYSC, RST,
> CPG, and the required clock descriptions.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 

Thanks, applied for v4.18.

Re: [PATCH/RFT v2 0/3] thermal: add support for r8a77995

[Simon Horman]

On Tue, Apr 03, 2018 at 08:00:50PM +0900, Yoshihiro Kaneko wrote:
> Hi Simon-san,
> 
> 2018-03-30 22:49 GMT+09:00 Simon Horman :
> > On Fri, Mar 30, 2018 at 12:13:00PM +0900, Yoshihiro Kaneko wrote:
> >> This series adds thermal support for r8a77995.
> >> R-Car D3 (r8a77995) have a thermal sensor module which is similar to Gen2.
> >> Therefore this series adds r8a77995 support to rcar_thermal driver not
> >> rcar_gen3_thermal driver.
> >>
> >> This series is based on the next branch of Zhang Rui's linux tree.
> >
> > I have very lightly tested this as follows after enabling
> > CONFIG_RCAR_THERMAL in the kernel .config.
> >
> > # cat /sys/devices/virtual/thermal/thermal_zone0/temp
> > 4
> 
> Thanks for the testing.
> Probably 40C is reasonable, is not?

Yes, probably.

Niklas do you have any guidance here?

Re: [PATCH 4/4] usb: gadget: udc: renesas_usb3: should call devm_phy_get() before add udc

[Simon Horman]

On Mon, Apr 02, 2018 at 09:21:34PM +0900, Yoshihiro Shimoda wrote:
> This patch fixes an issue that this driver cannot call phy_init()
> if a gadget driver is alreadly loaded because usb_add_gadget_udc()
> might call renesas_usb3_start() via .udc_start.
> 
> Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic 
> phy")
> Cc:  # v4.15+
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/usb/gadget/udc/renesas_usb3.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Simon Horman 


Please see some suggestions for follow-up lower-priority patches below.

> 
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c 
> b/drivers/usb/gadget/udc/renesas_usb3.c
> index 738b734..671bac3 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -2632,6 +2632,14 @@ static int renesas_usb3_probe(struct platform_device 
> *pdev)
>   if (ret < 0)
>   goto err_alloc_prd;
>  
> + /*
> +  * This is an optional. So, if this driver cannot get a phy,
> +  * this driver will not handle a phy anymore.
> +  */

nit: s/an optional/optional/

> + usb3->phy = devm_phy_get(>dev, "usb");
> + if (IS_ERR(usb3->phy))
> + usb3->phy = NULL;

I think this will unintentionally ignore errors other than the
non-existence of the device, f.e. a memory allocation failure
in devm_phy_get().

So perhaps something like this, as per phy_optional_get(), would be better?

usb3->phy = devm_phy_get(>dev, "usb");
if (IS_ERR(usb3->phy) && (PTR_ERR(usb3->phy) == -ENODEV))
usb3->phy = NULL;

> +
>   pm_runtime_enable(>dev);
>   ret = usb_add_gadget_udc(>dev, >gadget);
>   if (ret < 0)
> @@ -2641,14 +2649,6 @@ static int renesas_usb3_probe(struct platform_device 
> *pdev)
>   if (ret < 0)
>   goto err_dev_create;
>  
> - /*
> -  * This is an optional. So, if this driver cannot get a phy,
> -  * this driver will not handle a phy anymore.
> -  */
> - usb3->phy = devm_phy_get(>dev, "usb");
> - if (IS_ERR(usb3->phy))
> - usb3->phy = NULL;
> -
>   usb3->workaround_for_vbus = priv->workaround_for_vbus;
>  
>   renesas_usb3_debugfs_init(usb3, >dev);
> -- 
> 1.9.1
> 

Re: [PATCH 3/4] usb: gadget: udc: renesas_usb3: should call pm_runtime_enable() before add udc

[Simon Horman]

On Mon, Apr 02, 2018 at 09:21:33PM +0900, Yoshihiro Shimoda wrote:
> This patch fixes an issue that this driver causes panic if a gadget
> driver is already loaded because usb_add_gadget_udc() might call
> renesas_usb3_start() via .udc_start, and then pm_runtime_get_sync()
> in renesas_usb3_start() doesn't work correctly.
> Note that the usb3_to_dev() macro should not be called at this timing
> because the macro uses the gadget structure.
> 
> Fixes: cf06df3fae28 ("usb: gadget: udc: renesas_usb3: move 
> pm_runtime_{en,dis}able()")
> Cc:  # v4.15+
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Simon Horman 

Re: [PATCH 2/4] usb: gadget: udc: renesas_usb3: should remove debugfs

[Simon Horman]

On Mon, Apr 02, 2018 at 09:21:32PM +0900, Yoshihiro Shimoda wrote:
> This patch fixes an issue that this driver doesn't remove its debugfs.
> 
> Fixes: 43ba968b00ea ("usb: gadget: udc: renesas_usb3: add debugfs to set the 
> b-device mode")
> Cc:  # v4.14+
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Simon Horman 

Re: [PATCH 1/4] usb: gadget: udc: renesas_usb3: fix double phy_put()

[Simon Horman]

On Mon, Apr 02, 2018 at 09:21:31PM +0900, Yoshihiro Shimoda wrote:
> This patch fixes an issue that this driver cause double phy_put()
> calling. This driver must not call phy_put() in the remove because
> the driver calls devm_phy_get() in the probe.
> 
> Fixes: 279d4bc64060 ("usb: gadget: udc: renesas_usb3: add support for generic 
> phy")
> Cc:  # v4.15+
> Signed-off-by: Yoshihiro Shimoda 

Reviewed-by: Simon Horman 

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Geert Uytterhoeven]

Hi Simon, Marek,

On Mon, Apr 9, 2018 at 1:41 PM, Simon Horman  wrote:
> On Mon, Apr 09, 2018 at 10:20:05AM +0200, Marek Vasut wrote:
>> On 04/09/2018 10:07 AM, Geert Uytterhoeven wrote:
>> > On Sun, Apr 8, 2018 at 3:09 PM, Marek Vasut  wrote:
>> >> From: Dien Pham 
>> >>
>> >> The controller clock can be switched off during suspend/resume,
>> >> let runtime PM take care of that.
>> >>
>> >> Signed-off-by: Dien Pham 
>> >> Signed-off-by: Hien Dang 
>> >> Signed-off-by: Marek Vasut 
>> >> Cc: Geert Uytterhoeven 
>> >> Cc: Lorenzo Pieralisi 
>> >> Cc: Phil Edworthy 
>> >> Cc: Simon Horman 
>> >> Cc: Wolfram Sang 
>> >> Cc: linux-renesas-soc@vger.kernel.org
>> >> To: linux-...@vger.kernel.org
>> >> ---
>> >> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
>> >>   reordering of function calls in probe
>> >> - Dispose of fail_clk in rcar_pcie_get_resources()
>> >> V3: - Fix up the failpath in probe function
>> >> V4: - Rebase on recent linux-next
>> >> V5: - Do not call pci_free_resource_list(>resources) if
>> >>   rcar_pcie_parse_request_of_pci_ranges() fails, since that
>> >>   functiona calls pci_free_resource_list() already.
>> >
>> > Thanks for the update!
>> >
>> >> --- a/drivers/pci/host/pcie-rcar.c
>> >> +++ b/drivers/pci/host/pcie-rcar.c
>> >
>> >> @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct platform_device 
>> >> *pdev)
>> >> if (err)
>> >> goto err_free_bridge;
>> >>
>> >> +   pm_runtime_enable(pcie->dev);
>> >> +   err = pm_runtime_get_sync(pcie->dev);
>> >> +   if (err < 0) {
>> >> +   dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
>> >> +   goto err_pm_disable;
>> >> +   }
>> >> +
>> >
>> > As you moved the pm_runtime setup up...
>> >
>> >> err = rcar_pcie_get_resources(pcie);
>> >> if (err < 0) {
>> >> dev_err(dev, "failed to request resources: %d\n", err);
>> >> -   goto err_free_resource_list;
>> >> +   goto err_pm_put;
>> >> }
>> >>
>> >> err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>> >> if (err)
>> >> -   goto err_free_resource_list;
>> >> -
>> >> -   pm_runtime_enable(dev);
>> >> -   err = pm_runtime_get_sync(dev);
>> >> -   if (err < 0) {
>> >> -   dev_err(dev, "pm_runtime_get_sync failed\n");
>> >> -   goto err_pm_disable;
>> >> -   }
>> >> +   goto err_pm_put;
>> >>
>> >> /* Failure to get a link might just be that no cards are inserted 
>> >> */
>> >> hw_init_fn = of_device_get_match_data(dev);
>> >> @@ -1174,9 +1161,8 @@ static int rcar_pcie_probe(struct platform_device 
>> >> *pdev)
>> >>
>> >>  err_pm_disable:
>> >> pm_runtime_disable(dev);
>> >
>> > ... shouldn't it be moved down here, for symmetry?
>>
>> I am reasonably certain the failpath should be correct now. Did I still
>> miss something ?
>
> It looks correct to me too. Geert are Marek and I missing something?

Probably it will still work fine, but after this patch, Runtime PM is enabled
early, and disabled early, which is not symmetrical.

I like symmetry ;-)

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] pwm: rcar: simplify getting .drvdata

[Simon Horman]

On Thu, Apr 05, 2018 at 07:25:51PM +0200, Wolfram Sang wrote:
> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang 
> ---
> 
> Only build tested. Fixed numerous times in other drivers, however...

Reviewed-by: Simon Horman 

Re: [PATCH v2 5/5] DT: pci: rcar-pci: document R8A77980 bindings

[Simon Horman]

On Sun, Apr 08, 2018 at 09:10:43PM +0300, Sergei Shtylyov wrote:
> Document the R-Car V3H (R8A77980) SoC in the R-Car PCIe bindings.
> 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Simon Horman 

Re: [PATCH V4] PCI: rcar: Use runtime PM to control controller clock

[Simon Horman]

On Sun, Apr 01, 2018 at 05:02:07PM +0200, Marek Vasut wrote:
> On 03/19/2018 11:34 AM, Simon Horman wrote:
> > On Mon, Mar 19, 2018 at 10:54:47AM +0100, Marek Vasut wrote:
> >> On 03/19/2018 09:44 AM, Simon Horman wrote:
> >>> On Sun, Mar 18, 2018 at 11:52:09AM +0100, Marek Vasut wrote:
>  From: Dien Pham 
> 
>  The controller clock can be switched off during suspend/resume,
>  let runtime PM take care of that.
> 
>  Signed-off-by: Dien Pham 
>  Signed-off-by: Hien Dang 
>  Signed-off-by: Marek Vasut 
>  Cc: Geert Uytterhoeven 
>  Cc: Lorenzo Pieralisi 
>  Cc: Phil Edworthy 
>  Cc: Simon Horman 
>  Cc: Wolfram Sang 
>  Cc: linux-renesas-soc@vger.kernel.org
>  To: linux-...@vger.kernel.org
>  ---
>  V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
>    reordering of function calls in probe
>  - Dispose of fail_clk in rcar_pcie_get_resources()
>  V3: - Fix up the failpath in probe function
>  V4: - Rebase on recent linux-next
>  ---
>   drivers/pci/host/pcie-rcar.c | 40 
>  
>   1 file changed, 12 insertions(+), 28 deletions(-)
> 
>  diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
>  index b4c4aad2cf66..93d59f15c589 100644
>  --- a/drivers/pci/host/pcie-rcar.c
>  +++ b/drivers/pci/host/pcie-rcar.c
>  @@ -142,7 +142,6 @@ struct rcar_pcie {
>   void __iomem*base;
>   struct list_headresources;
>   int root_bus_nr;
>  -struct clk  *clk;
>   struct clk  *bus_clk;
>   struct  rcar_msi msi;
>   };
>  @@ -914,24 +913,14 @@ static int rcar_pcie_get_resources(struct 
>  rcar_pcie *pcie)
>   if (IS_ERR(pcie->base))
>   return PTR_ERR(pcie->base);
>   
>  -pcie->clk = devm_clk_get(dev, "pcie");
>  -if (IS_ERR(pcie->clk)) {
>  -dev_err(dev, "cannot get platform clock\n");
>  -return PTR_ERR(pcie->clk);
>  -}
>  -err = clk_prepare_enable(pcie->clk);
>  -if (err)
>  -return err;
>  -
>   pcie->bus_clk = devm_clk_get(dev, "pcie_bus");
>   if (IS_ERR(pcie->bus_clk)) {
>   dev_err(dev, "cannot get pcie bus clock\n");
>  -err = PTR_ERR(pcie->bus_clk);
>  -goto fail_clk;
>  +return PTR_ERR(pcie->bus_clk);
>   }
>   err = clk_prepare_enable(pcie->bus_clk);
>   if (err)
>  -goto fail_clk;
>  +return err;
>   
>   i = irq_of_parse_and_map(dev->of_node, 0);
>   if (!i) {
>  @@ -953,8 +942,6 @@ static int rcar_pcie_get_resources(struct rcar_pcie 
>  *pcie)
>   
>   err_map_reg:
>   clk_disable_unprepare(pcie->bus_clk);
>  -fail_clk:
>  -clk_disable_unprepare(pcie->clk);
>   
>   return err;
>   }
>  @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct 
>  platform_device *pdev)
>   if (err)
>   goto err_free_bridge;
> >>>
> >>> This error path now calls pci_free_resource_list() and in this case
> >>> the path is taken if rcar_pcie_parse_request_of_pci_ranges() fails.
> >>> Is that ok?
> >>
> >> I think so, or did I miss something obvious?
> > 
> > I feel that I am the one missing something obvious.
> > But here is what I am worried about:
> > 
> > 1. rcar_pcie_probe() calls rcar_pcie_parse_request_of_pci_ranges()
> > 2. rcar_pcie_parse_request_of_pci_ranges() calls
> >devm_request_pci_bus_resources() but that fails so
> >pci_free_resource_list() is called and an error is returned.
> > 3. rcar_pcie_probe() branches to err_free_bridge and calls
> >pci_free_resource_list() again.
> 
> I see what you mean now, thanks for the details explanation.
> 
> So the change needed here is to move
> pci_free_resource_list(>resources); above err_free_bridge, so it's
> not called twice, right ?

Yes, I think so.

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Simon Horman]

On Mon, Apr 09, 2018 at 10:20:05AM +0200, Marek Vasut wrote:
> On 04/09/2018 10:07 AM, Geert Uytterhoeven wrote:
> > Hi Marek,
> > 
> > On Sun, Apr 8, 2018 at 3:09 PM, Marek Vasut  wrote:
> >> From: Dien Pham 
> >>
> >> The controller clock can be switched off during suspend/resume,
> >> let runtime PM take care of that.
> >>
> >> Signed-off-by: Dien Pham 
> >> Signed-off-by: Hien Dang 
> >> Signed-off-by: Marek Vasut 
> >> Cc: Geert Uytterhoeven 
> >> Cc: Lorenzo Pieralisi 
> >> Cc: Phil Edworthy 
> >> Cc: Simon Horman 
> >> Cc: Wolfram Sang 
> >> Cc: linux-renesas-soc@vger.kernel.org
> >> To: linux-...@vger.kernel.org
> >> ---
> >> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
> >>   reordering of function calls in probe
> >> - Dispose of fail_clk in rcar_pcie_get_resources()
> >> V3: - Fix up the failpath in probe function
> >> V4: - Rebase on recent linux-next
> >> V5: - Do not call pci_free_resource_list(>resources) if
> >>   rcar_pcie_parse_request_of_pci_ranges() fails, since that
> >>   functiona calls pci_free_resource_list() already.
> > 
> > Thanks for the update!
> > 
> >> --- a/drivers/pci/host/pcie-rcar.c
> >> +++ b/drivers/pci/host/pcie-rcar.c
> > 
> >> @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct platform_device 
> >> *pdev)
> >> if (err)
> >> goto err_free_bridge;
> >>
> >> +   pm_runtime_enable(pcie->dev);
> >> +   err = pm_runtime_get_sync(pcie->dev);
> >> +   if (err < 0) {
> >> +   dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
> >> +   goto err_pm_disable;
> >> +   }
> >> +
> > 
> > As you moved the pm_runtime setup up...
> > 
> >> err = rcar_pcie_get_resources(pcie);
> >> if (err < 0) {
> >> dev_err(dev, "failed to request resources: %d\n", err);
> >> -   goto err_free_resource_list;
> >> +   goto err_pm_put;
> >> }
> >>
> >> err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
> >> if (err)
> >> -   goto err_free_resource_list;
> >> -
> >> -   pm_runtime_enable(dev);
> >> -   err = pm_runtime_get_sync(dev);
> >> -   if (err < 0) {
> >> -   dev_err(dev, "pm_runtime_get_sync failed\n");
> >> -   goto err_pm_disable;
> >> -   }
> >> +   goto err_pm_put;
> >>
> >> /* Failure to get a link might just be that no cards are inserted 
> >> */
> >> hw_init_fn = of_device_get_match_data(dev);
> >> @@ -1174,9 +1161,8 @@ static int rcar_pcie_probe(struct platform_device 
> >> *pdev)
> >>
> >>  err_pm_disable:
> >> pm_runtime_disable(dev);
> > 
> > ... shouldn't it be moved down here, for symmetry?
> 
> I am reasonably certain the failpath should be correct now. Did I still
> miss something ?

It looks correct to me too. Geert are Marek and I missing something?

Re: [PATCH V3] PCI: rcar: Clean up the macros

[Simon Horman]

On Sun, Apr 08, 2018 at 08:04:31PM +0200, Marek Vasut wrote:
> This patch replaces the (1 << n) with BIT(n) and cleans up whitespace,
> no functional change.
> 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> Reviewed-by: Niklas Söderlund 

Reviewed-by: Simon Horman 

Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()

[Simon Horman]

On Mon, Apr 09, 2018 at 12:54:18PM +0200, Simon Horman wrote:
> On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> > In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> > for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> > the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?
> 
> If so can we consider a helper. rcar_pcie_wait_for_phyrdy() looks
> very similar to that R-Car H1 code.
> 
> > Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
> > special PHY driver on the R-Car V3H the PCIEC initialization just freezes
> > the kernel --  adding the PHYRDY polling allows the init code to exit
> > gracefully on timeout (PHY starts powered down after reset on this SoC).
> 
> How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.
> 
> > 
> > Signed-off-by: Sergei Shtylyov 
> > 
> > ---
> >  drivers/pci/host/pcie-rcar.c |   20 
> >  1 file changed, 20 insertions(+)
> > 
> > Index: pci/drivers/pci/host/pcie-rcar.c
> > ===
> > --- pci.orig/drivers/pci/host/pcie-rcar.c
> > +++ pci/drivers/pci/host/pcie-rcar.c
> > @@ -36,6 +36,8 @@
> >  #define PCIECDR0x20
> >  #define PCIEMSR0x28
> >  #define PCIEINTXR  0x000400
> > +#define PCIEPHYSR  0x0007f0
> > +#define  PHYRDY1
> 
> Can we start using the BIT() macro in this driver?

I see this is handled by

[PATCH V3] PCI: rcar: Clean up the macros

> 
> >  #define PCIEMSITXR 0x000840
> >  
> >  /* Transfer control */
> > @@ -527,6 +529,20 @@ static void phy_write_reg(struct rcar_pc
> > phy_wait_for_ack(pcie);
> >  }
> >  
> > +static int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
> > +{
> > +   unsigned int timeout = 10;
> > +
> > +   while (timeout--) {
> > +   if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
> > +   return 0;
> > +
> > +   msleep(5);
> > +   }
> > +
> > +   return -ETIMEDOUT;
> > +}
> > +
> >  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
> >  {
> > unsigned int timeout = 10;
> > @@ -551,6 +567,10 @@ static int rcar_pcie_hw_init(struct rcar
> > /* Set mode */
> > rcar_pci_write_reg(pcie, 1, PCIEMSR);
> >  
> > +   err = rcar_pcie_wait_for_phyrdy(pcie);
> > +   if (err)
> > +   return err;
> > +
> > /*
> >  * Initial header for port config space is type 1, set the device
> >  * class to match. Hardware takes care of propagating the IDSETR
> > 
> 

Re: [PATCH v5 0/3] R-Car DU: Fix LVDS output on Gen2 boards

[Simon Horman]

On Fri, Apr 06, 2018 at 10:14:35PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series fixes LVDS output support on the Lager, Koelsh, Porter and
> Gose boards that broke in v4.17-rc1 due to the combination of the R-Car DU
> LVDS driver rework and the DT move of all on-SoC peripherals to a /soc node.
> 
> We could handle the problem in the R-Car DU LVDS DT backward compatibility
> code, but that fix would only be used for v4.17 as in v4.18 the Gen2 DT will
> move to the new LVDS DT bindings. I thus propose merging these three patches
> in v4.17 already to fix the problem as this is the simplest solution.
> 
> The patches are based on top of Linus' master that includes both the R-Car DU
> changes and the ARM DT changes for v4.17-rc1. I can rebase them on top of
> v4.17-rc1 when it will be released, but I don't expect any change.

Thanks, I think this should be fine. Please ping me once v4.17-rc1 has
been released.

> 
> Laurent Pinchart (3):
>   ARM: dts: r8a7790: Convert to new LVDS DT bindings
>   ARM: dts: r8a7791: Convert to new LVDS DT bindings
>   ARM: dts: r8a7793: Convert to new LVDS DT bindings
> 
>  arch/arm/boot/dts/r8a7790-lager.dts   | 22 +---
>  arch/arm/boot/dts/r8a7790.dtsi| 65 
> ++-
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 10 --
>  arch/arm/boot/dts/r8a7791-porter.dts  | 16 +++--
>  arch/arm/boot/dts/r8a7791.dtsi| 36 +++
>  arch/arm/boot/dts/r8a7793-gose.dts| 10 --
>  arch/arm/boot/dts/r8a7793.dtsi| 37 
>  7 files changed, 163 insertions(+), 33 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

Re: [RFC v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()

[Robin Murphy]

Hi Jacopo,

On 09/04/18 08:25, jacopo mondi wrote:

Hi Robin, Laurent,
 a long time passed, sorry about this.

On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote:

On 14/11/17 17:08, Jacopo Mondi wrote:

On SH4 architecture, with SPARSEMEM memory model, translating page to
pfn hangs the CPU. Post-pone translation to pfn after
dma_mmap_from_dev_coherent() function call as it succeeds and make page
translation not necessary.

This patch was suggested by Laurent Pinchart and he's working to submit
a proper fix mainline. Not sending for inclusion at the moment.


Y'know, I think this patch does have some merit by itself - until we know
that cpu_addr *doesn't* represent some device-private memory which is not
guaranteed to be backed by a struct page, calling virt_to_page() on it is
arguably semantically incorrect, even if it might happen to be benign in
most cases.


I still need to carry this patch in my trees to have a working dma memory 
mapping
on SH4 platforms. My understanding from your comment is that there may
be a way forward for this patch, do you still think the same? Have you
got any suggestion on how to improve this eventually for inclusion?


As before, the change itself does seem reasonable; it might be worth 
rewording the commit message in more general terms rather than making it 
sound like an SH-specific workaround (which I really don't think it is), 
but otherwise I'd say just repost it as a non-RFC patch.


Robin.



Thanks
j



Robin.


Suggested-by: Laurent Pinchart 
Signed-off-by: Jacopo Mondi 
---
  drivers/base/dma-mapping.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
index e584edd..73d64d3 100644
--- a/drivers/base/dma-mapping.c
+++ b/drivers/base/dma-mapping.c
@@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct 
vm_area_struct *vma,
  #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
unsigned long user_count = vma_pages(vma);
unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
-   unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
unsigned long off = vma->vm_pgoff;
+   unsigned long pfn;

vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

@@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct 
vm_area_struct *vma,
return ret;

if (off < count && user_count <= (count - off)) {
+   pfn = page_to_pfn(virt_to_page(cpu_addr));
ret = remap_pfn_range(vma, vma->vm_start,
  pfn + off,
  user_count << PAGE_SHIFT,
--
2.7.4

___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call

[Simon Horman]

On Fri, Apr 06, 2018 at 02:10:22PM +0300, Sergei Shtylyov wrote:
> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
> makes  sense to move that call into the driver's probe() method and then
> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...

I'm not sure the churn is worth it, but if you do then that is find by me.

> Signed-off-by: Sergei Shtylyov 
> 
> ---
>  drivers/pci/host/pcie-rcar.c |   42 
> ++
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> Index: pci/drivers/pci/host/pcie-rcar.c
> ===
> --- pci.orig/drivers/pci/host/pcie-rcar.c
> +++ pci/drivers/pci/host/pcie-rcar.c
> @@ -626,7 +626,7 @@ static int rcar_pcie_hw_init(struct rcar
>   return 0;
>  }
>  
> -static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_h1(struct rcar_pcie *pcie)
>  {
>   /* Initialize the phy */
>   phy_write_reg(pcie, 0, 0x42, 0x1, 0x0EC34191);
> @@ -646,10 +646,10 @@ static int rcar_pcie_hw_init_h1(struct r
>   phy_write_reg(pcie, 0, 0x64, 0x1, 0x3F0F1F0F);
>   phy_write_reg(pcie, 0, 0x66, 0x1, 0x8000);
>  
> - return rcar_pcie_hw_init(pcie);
> + return 0;
>  }
>  
> -static int rcar_pcie_hw_init_gen2(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_gen2(struct rcar_pcie *pcie)
>  {
>   /*
>* These settings come from the R-Car Series, 2nd Generation User's
> @@ -666,10 +666,10 @@ static int rcar_pcie_hw_init_gen2(struct
>   rcar_pci_write_reg(pcie, 0x0001, GEN2_PCIEPHYCTRL);
>   rcar_pci_write_reg(pcie, 0x0006, GEN2_PCIEPHYCTRL);
>  
> - return rcar_pcie_hw_init(pcie);
> + return 0;
>  }
>  
> -static int rcar_pcie_hw_init_gen3(struct rcar_pcie *pcie)
> +static int rcar_pcie_phy_init_gen3(struct rcar_pcie *pcie)
>  {
>   int err;
>  
> @@ -677,11 +677,7 @@ static int rcar_pcie_hw_init_gen3(struct
>   if (err)
>   return err;
>  
> - err = phy_power_on(pcie->phy);
> - if (err)
> - return err;
> -
> - return rcar_pcie_hw_init(pcie);
> + return phy_power_on(pcie->phy);
>  }
>  
>  static int rcar_msi_alloc(struct rcar_msi *chip)
> @@ -1082,17 +1078,18 @@ static int rcar_pcie_parse_map_dma_range
>  }
>  
>  static const struct of_device_id rcar_pcie_of_match[] = {
> - { .compatible = "renesas,pcie-r8a7779", .data = rcar_pcie_hw_init_h1 },
> + { .compatible = "renesas,pcie-r8a7779",
> +   .data = rcar_pcie_phy_init_h1 },
>   { .compatible = "renesas,pcie-r8a7790",
> -   .data = rcar_pcie_hw_init_gen2 },
> +   .data = rcar_pcie_phy_init_gen2 },
>   { .compatible = "renesas,pcie-r8a7791",
> -   .data = rcar_pcie_hw_init_gen2 },
> +   .data = rcar_pcie_phy_init_gen2 },
>   { .compatible = "renesas,pcie-rcar-gen2",
> -   .data = rcar_pcie_hw_init_gen2 },
> +   .data = rcar_pcie_phy_init_gen2 },
>   { .compatible = "renesas,pcie-r8a7795",
> -   .data = rcar_pcie_hw_init_gen3 },
> +   .data = rcar_pcie_phy_init_gen3 },
>   { .compatible = "renesas,pcie-rcar-gen3",
> -   .data = rcar_pcie_hw_init_gen3 },
> +   .data = rcar_pcie_phy_init_gen3 },
>   {},

I would avoid the line wrapping here, but its up to you.

>  };
>  
> @@ -1140,7 +1137,7 @@ static int rcar_pcie_probe(struct platfo
>   struct rcar_pcie *pcie;
>   unsigned int data;
>   int err;
> - int (*hw_init_fn)(struct rcar_pcie *);
> + int (*phy_init_fn)(struct rcar_pcie *);

Looking at this I wonder if we also need a phy_cleanup() code or
similar.

>   struct pci_host_bridge *bridge;
>  
>   bridge = pci_alloc_host_bridge(sizeof(*pcie));
> @@ -1174,10 +1171,15 @@ static int rcar_pcie_probe(struct platfo
>   goto err_pm_disable;
>   }
>  
> - /* Failure to get a link might just be that no cards are inserted */
> - hw_init_fn = of_device_get_match_data(dev);
> - err = hw_init_fn(pcie);
> + phy_init_fn = of_device_get_match_data(dev);
> + err = phy_init_fn(pcie);
>   if (err) {
> + dev_err(dev, "failed to init PCIe PHY\n");
> + goto err_pm_put;
> + }
> +
> + /* Failure to get a link might just be that no cards are inserted */
> + if (rcar_pcie_hw_init(pcie)) {
>   dev_info(dev, "PCIe link down\n");
>   err = -ENODEV;
>   goto err_pm_put;
> 

Re: [PATCH 3/4] pcie-rcar: add R-Car gen3 PHY support

[Simon Horman]

On Fri, Apr 06, 2018 at 02:08:12PM +0300, Sergei Shtylyov wrote:
> On R-Car gen3 SoCs the PCIe PHY has its own register region -- and I have
> written  a generic PHY driver for it, thus we need to add the corresponding
> code in  rcar_pcie_hw_init_gen3() and call devm_phy_optional_get() at the
> driver's probing time,  so that the existing R-Car gen3 device trees (not
> having a PHY node) would still work (we only need  to power up the PHY on
> R-Car V3H).
> 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Simon Horman 

Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()

[Simon Horman]

On Mon, Apr 09, 2018 at 12:54:18PM +0200, Simon Horman wrote:
> On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> > In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> > for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> > the driver only does this on R-Car H1 (polling a PHY specific register).
> 
> Is the R-Car H1 specific code still needed with this patch in place?

Sorry, I now see you addressed that by removing the R-Car H1 code in patch 2/4.

...

Re: [PATCH 2/4] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()

[Simon Horman]

On Fri, Apr 06, 2018 at 02:04:52PM +0300, Sergei Shtylyov wrote:
> Now that we've added PCIEPHYSR.PHYRDY polling to rcar_pcie_hw_init(),
> there is no need anymore  for polling the PHY specific register in
> rcar_pcie_hw_init_h1() -- remove it.
> 
> Signed-off-by: Sergei Shtylyov 

This looks good, but has it been tested?

Re: [PATCH 1/4] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()

[Simon Horman]

On Fri, Apr 06, 2018 at 02:02:52PM +0300, Sergei Shtylyov wrote:
> In  all the R-Car gen1/2/3 manuals, we are instructed to poll PCIEPHYSR
> for PHYRDY=1  at  an early stage of the PCIEC initialization -- while
> the driver only does this on R-Car H1 (polling a PHY specific register).

Is the R-Car H1 specific code still needed with this patch in place?

If so can we consider a helper. rcar_pcie_wait_for_phyrdy() looks
very similar to that R-Car H1 code.

> Add the PHYRDY polling to rcar_pcie_hw_init(). Note that without the
> special PHY driver on the R-Car V3H the PCIEC initialization just freezes
> the kernel --  adding the PHYRDY polling allows the init code to exit
> gracefully on timeout (PHY starts powered down after reset on this SoC).

How widely has this been exercised? I assume it affects Rcar Gen 1, 2 and 3.

> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
>  drivers/pci/host/pcie-rcar.c |   20 
>  1 file changed, 20 insertions(+)
> 
> Index: pci/drivers/pci/host/pcie-rcar.c
> ===
> --- pci.orig/drivers/pci/host/pcie-rcar.c
> +++ pci/drivers/pci/host/pcie-rcar.c
> @@ -36,6 +36,8 @@
>  #define PCIECDR  0x20
>  #define PCIEMSR  0x28
>  #define PCIEINTXR0x000400
> +#define PCIEPHYSR0x0007f0
> +#define  PHYRDY  1

Can we start using the BIT() macro in this driver?

>  #define PCIEMSITXR   0x000840
>  
>  /* Transfer control */
> @@ -527,6 +529,20 @@ static void phy_write_reg(struct rcar_pc
>   phy_wait_for_ack(pcie);
>  }
>  
> +static int rcar_pcie_wait_for_phyrdy(struct rcar_pcie *pcie)
> +{
> + unsigned int timeout = 10;
> +
> + while (timeout--) {
> + if (rcar_pci_read_reg(pcie, PCIEPHYSR) & PHYRDY)
> + return 0;
> +
> + msleep(5);
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>  {
>   unsigned int timeout = 10;
> @@ -551,6 +567,10 @@ static int rcar_pcie_hw_init(struct rcar
>   /* Set mode */
>   rcar_pci_write_reg(pcie, 1, PCIEMSR);
>  
> + err = rcar_pcie_wait_for_phyrdy(pcie);
> + if (err)
> + return err;
> +
>   /*
>* Initial header for port config space is type 1, set the device
>* class to match. Hardware takes care of propagating the IDSETR
> 

Re: [PATCH v7 1/2] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Mark Brown]

On Fri, Apr 06, 2018 at 06:40:14PM +0300, Laurent Pinchart wrote:
> On Friday, 6 April 2018 17:25:58 EEST jacopo mondi wrote:

> > Same on the mandatory/optional VCC supply thing. Let's try to make
> > next version the final one. If the optional property with the dummy
> > regulator doesn't satisfy you and it is preferred to have a fixed-regulator
> > anyhow in DT I'll do in next version, othewise let's try not to change
> > it again. I'll just remark here that in the current Eagle design vcc is
> > connected to a power rail with no regulator at all :)

> I don't like the dummy regulator much, as it generates a dev_warn(), which 
> makes me believe that it's a hack rather than a proper solution. You might 
> want to ask Mark Brown for his opinion.

The DT should describe the physical supplies the device needs, if that's
a fixed regulator with no software control then the DT should have a
fixed regulator with no software control.


signature.asc
Description: PGP signature

Re: [PATCH] phy: Renesas R-Car gen3 PCIe PHY driver

[Geert Uytterhoeven]

Hi Sergei,

Thanks for your patch!

On Wed, Apr 4, 2018 at 9:31 PM, Sergei Shtylyov
 wrote:
> This PHY is still mostly undocumented -- the only documented registers
> exist on R-Car V3H (R8A77980) SoC  where this PHY stays in a powered-down
> state after a reset and thus  we must power it on for PCIe to work...

Bogus spaces slipping in again?

> Signed-off-by: Sergei Shtylyov 

> --- /dev/null
> +++ linux-phy/Documentation/devicetree/bindings/phy/rcar-gen3-phy-pcie.txt
> @@ -0,0 +1,32 @@
> +* Renesas R-Car generation 3 PCIe PHY
> +
> +This file provides information on what the device node for the R-Car
> +generation 3 PCIe PHY contains.
> +
> +Required properties:
> +- compatible: "renesas,r8a77980-pcie-phy" if the device is a part of R8A77980
> + SoC.
> + "renesas,rcar-gen3-pcie-phy" for a generic R-Car Gen3 compatible
> + device.
> +
> + When compatible with the generic version, nodes must list the
> + SoC-specific version corresponding to the platform first
> + followed by the generic version.
> +
> +- reg: offset and length of the register block.
> +- clocks: clock phandle and specifier pair.
> +- power-domains: power domain phandle and specifier pair.
> +- resets: reset phandle and specifier pair.
> +- #phy-cells: see phy-bindings.txt in the same directory, must be <0>.
> +
> +Example (R-Car V3H):
> +
> +   pcie-phy@e65d {
> +   compatible = "renesas,r8a77980-pcie-phy",
> +"renesas,rcar-gen3-pcie-phy";

Is the R8A77980 PCIe PHY compatible with the generic version, given it needs
to be powered up explicitly?

The only documented register is the lane reversal register, do we need bindings
to configure it?

> --- /dev/null
> +++ linux-phy/drivers/phy/renesas/phy-rcar-gen3-pcie.c
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas R-Car Gen2 PHY driver

Gen3 PCIe PHY

> +static int rcar_gen3_phy_pcie_probe(struct platform_device *pdev)
> +{
> +   struct device *dev = >dev;
> +   struct phy_provider *provider;
> +   struct rcar_gen3_phy *phy;
> +   struct resource *res;
> +   void __iomem *base;
> +   int error;
> +
> +   if (!dev->of_node) {
> +   dev_err(dev,
> +   "This driver must only be instantiated from the 
> device tree\n");
> +   return -EINVAL;
> +   }

Do we need this check? Just go bang, like most other DT-only drivers do?

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 2/3] ARM: dts: r8a7791: Convert to new LVDS DT bindings

[Geert Uytterhoeven]

On Sun, Apr 8, 2018 at 6:40 PM, Niklas Söderlund
 wrote:
> On 2018-04-06 22:14:37 +0300, Laurent Pinchart wrote:
>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
>> @@ -470,8 +470,7 @@
>>
>>   clocks = < CPG_MOD 724>, < CPG_MOD 723>, < CPG_MOD 726>,
>><_clk>, <_clk>;
>
> I think you forgot to delete < CPG_MOD 726> from the clocks, with
> that fixed:
>
> Reviewed-by: Niklas Söderlund 
>
>> - clock-names = "du.0", "du.1", "lvds.0",
>> -   "dclkin.0", "dclkin.1";
>> + clock-names = "du.0", "du.1", "dclkin.0", "dclkin.1";

Obviously we need a check in dtc for matching the lengths of the clocks and
clock-names (etc.) properties.

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 4/4] pcie-rcar: factor out rcar_pcie_hw_init() call

[Geert Uytterhoeven]

On Fri, Apr 6, 2018 at 1:10 PM, Sergei Shtylyov
 wrote:
> We now have rcar_pcie_hw_init_{h1|gen2|gen3}() differing only in the PCIe
> PHY init code and all ending with a call to rcar_pcie_hw_init(), thus it
> makes  sense to move that call into the driver's probe() method and then
> rename those functions to rcar_pcie_phy_init_{h1|gen2|gen3}() -- doing
> this saves 48 bytes of object code (AArch64 gcc 4.8.5)...
>
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Geert Uytterhoeven 

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] pwm: rcar: simplify getting .drvdata

[Geert Uytterhoeven]

On Thu, Apr 5, 2018 at 7:25 PM, Wolfram Sang
 wrote:
> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.
>
> Signed-off-by: Wolfram Sang 

Reviewed-by: Geert Uytterhoeven 

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] PCI: rcar: Use runtime PM to control controller clock

[Marek Vasut]

On 04/09/2018 10:07 AM, Geert Uytterhoeven wrote:
> Hi Marek,
> 
> On Sun, Apr 8, 2018 at 3:09 PM, Marek Vasut  wrote:
>> From: Dien Pham 
>>
>> The controller clock can be switched off during suspend/resume,
>> let runtime PM take care of that.
>>
>> Signed-off-by: Dien Pham 
>> Signed-off-by: Hien Dang 
>> Signed-off-by: Marek Vasut 
>> Cc: Geert Uytterhoeven 
>> Cc: Lorenzo Pieralisi 
>> Cc: Phil Edworthy 
>> Cc: Simon Horman 
>> Cc: Wolfram Sang 
>> Cc: linux-renesas-soc@vger.kernel.org
>> To: linux-...@vger.kernel.org
>> ---
>> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
>>   reordering of function calls in probe
>> - Dispose of fail_clk in rcar_pcie_get_resources()
>> V3: - Fix up the failpath in probe function
>> V4: - Rebase on recent linux-next
>> V5: - Do not call pci_free_resource_list(>resources) if
>>   rcar_pcie_parse_request_of_pci_ranges() fails, since that
>>   functiona calls pci_free_resource_list() already.
> 
> Thanks for the update!
> 
>> --- a/drivers/pci/host/pcie-rcar.c
>> +++ b/drivers/pci/host/pcie-rcar.c
> 
>> @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct platform_device 
>> *pdev)
>> if (err)
>> goto err_free_bridge;
>>
>> +   pm_runtime_enable(pcie->dev);
>> +   err = pm_runtime_get_sync(pcie->dev);
>> +   if (err < 0) {
>> +   dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
>> +   goto err_pm_disable;
>> +   }
>> +
> 
> As you moved the pm_runtime setup up...
> 
>> err = rcar_pcie_get_resources(pcie);
>> if (err < 0) {
>> dev_err(dev, "failed to request resources: %d\n", err);
>> -   goto err_free_resource_list;
>> +   goto err_pm_put;
>> }
>>
>> err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>> if (err)
>> -   goto err_free_resource_list;
>> -
>> -   pm_runtime_enable(dev);
>> -   err = pm_runtime_get_sync(dev);
>> -   if (err < 0) {
>> -   dev_err(dev, "pm_runtime_get_sync failed\n");
>> -   goto err_pm_disable;
>> -   }
>> +   goto err_pm_put;
>>
>> /* Failure to get a link might just be that no cards are inserted */
>> hw_init_fn = of_device_get_match_data(dev);
>> @@ -1174,9 +1161,8 @@ static int rcar_pcie_probe(struct platform_device 
>> *pdev)
>>
>>  err_pm_disable:
>> pm_runtime_disable(dev);
> 
> ... shouldn't it be moved down here, for symmetry?

I am reasonably certain the failpath should be correct now. Did I still
miss something ?

-- 
Best regards,
Marek Vasut

Re: [PATCH V3] PCI: rcar: Clean up the macros

[Geert Uytterhoeven]

On Sun, Apr 8, 2018 at 8:04 PM, Marek Vasut  wrote:
> This patch replaces the (1 << n) with BIT(n) and cleans up whitespace,
> no functional change.
>
> Signed-off-by: Marek Vasut 

Reviewed-by: Geert Uytterhoeven 

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] PCI: rcar: Use runtime PM to control controller clock

[Geert Uytterhoeven]

Hi Marek,

On Sun, Apr 8, 2018 at 3:09 PM, Marek Vasut  wrote:
> From: Dien Pham 
>
> The controller clock can be switched off during suspend/resume,
> let runtime PM take care of that.
>
> Signed-off-by: Dien Pham 
> Signed-off-by: Hien Dang 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Lorenzo Pieralisi 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> To: linux-...@vger.kernel.org
> ---
> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
>   reordering of function calls in probe
> - Dispose of fail_clk in rcar_pcie_get_resources()
> V3: - Fix up the failpath in probe function
> V4: - Rebase on recent linux-next
> V5: - Do not call pci_free_resource_list(>resources) if
>   rcar_pcie_parse_request_of_pci_ranges() fails, since that
>   functiona calls pci_free_resource_list() already.

Thanks for the update!

> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c

> @@ -1124,22 +,22 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
> if (err)
> goto err_free_bridge;
>
> +   pm_runtime_enable(pcie->dev);
> +   err = pm_runtime_get_sync(pcie->dev);
> +   if (err < 0) {
> +   dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
> +   goto err_pm_disable;
> +   }
> +

As you moved the pm_runtime setup up...

> err = rcar_pcie_get_resources(pcie);
> if (err < 0) {
> dev_err(dev, "failed to request resources: %d\n", err);
> -   goto err_free_resource_list;
> +   goto err_pm_put;
> }
>
> err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
> if (err)
> -   goto err_free_resource_list;
> -
> -   pm_runtime_enable(dev);
> -   err = pm_runtime_get_sync(dev);
> -   if (err < 0) {
> -   dev_err(dev, "pm_runtime_get_sync failed\n");
> -   goto err_pm_disable;
> -   }
> +   goto err_pm_put;
>
> /* Failure to get a link might just be that no cards are inserted */
> hw_init_fn = of_device_get_match_data(dev);
> @@ -1174,9 +1161,8 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>
>  err_pm_disable:
> pm_runtime_disable(dev);

... shouldn't it be moved down here, for symmetry?

> -
> -err_free_resource_list:
> pci_free_resource_list(>resources);
> +
>  err_free_bridge:
> pci_free_host_bridge(bridge);

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 v2 2/2] base: dma-mapping: Postpone page_to_pfn() on mmap()

[jacopo mondi]

Hi Robin, Laurent,
a long time passed, sorry about this.

On Wed, Nov 15, 2017 at 01:38:23PM +, Robin Murphy wrote:
> On 14/11/17 17:08, Jacopo Mondi wrote:
> >On SH4 architecture, with SPARSEMEM memory model, translating page to
> >pfn hangs the CPU. Post-pone translation to pfn after
> >dma_mmap_from_dev_coherent() function call as it succeeds and make page
> >translation not necessary.
> >
> >This patch was suggested by Laurent Pinchart and he's working to submit
> >a proper fix mainline. Not sending for inclusion at the moment.
>
> Y'know, I think this patch does have some merit by itself - until we know
> that cpu_addr *doesn't* represent some device-private memory which is not
> guaranteed to be backed by a struct page, calling virt_to_page() on it is
> arguably semantically incorrect, even if it might happen to be benign in
> most cases.

I still need to carry this patch in my trees to have a working dma memory 
mapping
on SH4 platforms. My understanding from your comment is that there may
be a way forward for this patch, do you still think the same? Have you
got any suggestion on how to improve this eventually for inclusion?

Thanks
   j

>
> Robin.
>
> >Suggested-by: Laurent Pinchart 
> >Signed-off-by: Jacopo Mondi 
> >---
> >  drivers/base/dma-mapping.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> >index e584edd..73d64d3 100644
> >--- a/drivers/base/dma-mapping.c
> >+++ b/drivers/base/dma-mapping.c
> >@@ -227,8 +227,8 @@ int dma_common_mmap(struct device *dev, struct 
> >vm_area_struct *vma,
> >  #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
> > unsigned long user_count = vma_pages(vma);
> > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >-unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> > unsigned long off = vma->vm_pgoff;
> >+unsigned long pfn;
> >
> > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> >
> >@@ -236,6 +236,7 @@ int dma_common_mmap(struct device *dev, struct 
> >vm_area_struct *vma,
> > return ret;
> >
> > if (off < count && user_count <= (count - off)) {
> >+pfn = page_to_pfn(virt_to_page(cpu_addr));
> > ret = remap_pfn_range(vma, vma->vm_start,
> >   pfn + off,
> >   user_count << PAGE_SHIFT,
> >--
> >2.7.4
> >
> >___
> >iommu mailing list
> >io...@lists.linux-foundation.org
> >https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >


signature.asc
Description: PGP signature