Re: phandles using absolute paths in DT overlays

[Geert Uytterhoeven]

Hi David,

Thanks for your answer!

On Fri, Dec 7, 2018 at 2:44 AM David Gibson  wrote:
> On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
> > Some early revisions of SoCs may have hardware bugs that need to be
> > fixed up in DT.  Currently we are handling this by including DTS files
> > and fixing up nodes and properties, to create different DTB files for
> > different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
> >
> > As an alternative, I'm envisioning the use of DT overlays and the
> > fdtoverlay tool, in the hope of simplifying the generation of DTBs for
> > the various SoC/board combinations.
> >
> > Ideally, such DTBs would not contain symbols, to avoid inflating DTB
> > size.  Hence if fixup overlays would not contain symbolic references,
> > there would be no need for symbols.
> >
> > For anchors, the "&{/path/to/node@address}" syntax is working fine.
> > For phandles, while documented on
> > https://elinux.org/Device_Tree_Mysteries, and while working fine for the
> > non-overlay case, dtc seems to have issues interpreting the DTB:
> >
> > $ scripts/dtc/dtc -I dtb -O dts my.dtb  | less
> > : ERROR (property_name_chars):
> > /__fixups__:/path/to/node@deadbeef: Bad character '/' in property
> > name
> > ERROR: Input tree has errors, aborting (use -f to force output)
> >
> > With -f, the fixup generated seems to contain the expected value, though:
> >
> > __fixups__ {
> > /path/to/node@deadbeef = "/fragment@0/__overlay__:power-domains:0";
> > };
> >
> > When using ftdoverlay, the situation is worse:
> >
> > Failed to apply my.dtb (-1)
> >
> > Are these known issues?
>
> Unfortunately, this can't work with the current overlay format.  We
> have a specific syntax to allow the target of an overlay fragment to
> specified as a path, but phandle references by path won't work.
>
> The problem is that the encoding of the fixups node has the fixup
> target as a property name, and as the error says '/' is an illegal
> character in property names - for a bunch of good reasons, so we can't
> just go and remove that restriction.

For the uneducated, can you please explain why '/' is an illegal character,
and why it can't be special cased for phandle references by path in overlays?

> So to allow this we'd need to come up with a different encoding for
> path-targeted fixups and add support for it in the various pieces of
> the chain.
>
> One way to do that would be to entirely rework the overlay format to
> something more sensible, which would have a number of other benefits.

IC.

Thanks!

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 2/3] arm64: defconfig: Enable scu-simple-card driver

[Geert Uytterhoeven]

Hi Simon,

On Thu, Dec 6, 2018 at 10:58 PM Simon Horman  wrote:
> Enable the scu-simple-card which is used by
> the R-Car E3 (r8a77990) based Ebisu board.
>
> Signed-off-by: Simon Horman 
> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index f88190463481..9d0b42d96f03 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -491,6 +491,7 @@ CONFIG_SND_SOC_RT5514=m
>  CONFIG_SND_SOC_RT5514_SPI=m
>  CONFIG_SND_SOC_RT5645=m
>  CONFIG_SND_SIMPLE_CARD=m
> +CONFIG_SND_SIMPLE_SCU_CARD=y

Shouldn't this be "m"?

>  CONFIG_SND_AUDIO_GRAPH_CARD=m
>  CONFIG_I2C_HID=m
>  CONFIG_USB=y

Gr{oetje,eeting}s,

Geert

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

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

[PATCH 1/5] pwm: rcar: add rcar_pwm_calc_counter() to calculate PWMCNT value only

[Yoshihiro Shimoda]

To add a workaround in the future, this patch adds a function
rcar_pwm_calc_counter() from rcar_pwm_set_counter().
The rcar_pwm_calc_counter() calculates PWMCNT value only, and
rcar_pwm_set_counter() set the PWMCNT register by using
the calculated value. No change in behavior.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/pwm/pwm-rcar.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index a41812f..9cf4567 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -36,6 +36,7 @@ struct rcar_pwm_chip {
struct pwm_chip chip;
void __iomem *base;
struct clk *clk;
+   u32 pwmcnt;
 };
 
 static inline struct rcar_pwm_chip *to_rcar_pwm_chip(struct pwm_chip *chip)
@@ -102,8 +103,8 @@ static void rcar_pwm_set_clock_control(struct rcar_pwm_chip 
*rp,
rcar_pwm_write(rp, value, RCAR_PWMCR);
 }
 
-static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
-   int period_ns)
+static void rcar_pwm_calc_counter(struct rcar_pwm_chip *rp, int div,
+ int duty_ns, int period_ns)
 {
unsigned long long one_cycle, tmp;  /* 0.01 nanoseconds */
unsigned long clk_rate = clk_get_rate(rp->clk);
@@ -120,11 +121,17 @@ static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, 
int div, int duty_ns,
do_div(tmp, one_cycle);
ph = tmp & RCAR_PWMCNT_PH0_MASK;
 
+   rp->pwmcnt = cyc | ph;
+}
+
+static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp)
+{
/* Avoid prohibited setting */
-   if (cyc == 0 || ph == 0)
+   if ((rp->pwmcnt & RCAR_PWMCNT_CYC0_MASK) == 0 ||
+   (rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
return -EINVAL;
 
-   rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
+   rcar_pwm_write(rp, rp->pwmcnt, RCAR_PWMCNT);
 
return 0;
 }
@@ -159,7 +166,8 @@ static int rcar_pwm_config(struct pwm_chip *chip, struct 
pwm_device *pwm,
 
rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
 
-   ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
+   rcar_pwm_calc_counter(rp, div, duty_ns, period_ns);
+   ret = rcar_pwm_set_counter(rp);
if (!ret)
rcar_pwm_set_clock_control(rp, div);
 
-- 
1.9.1

[PATCH 0/5] pwm: rcar: Add support "atomic" API and workaround

[Yoshihiro Shimoda]

This patch adds support for PWM "atomic" API.

This patch also adds a workaround to output "pseudo" low level.
Otherwise, the PWM backlight driver doesn't work correctly, especially
it cannot output maximum brightness actually.

Yoshihiro Shimoda (5):
  pwm: rcar: add rcar_pwm_calc_counter() to calculate PWMCNT value only
  pwm: rcar: Add support "atomic" API
  pwm: rcar: Use "atomic" API on rcar_pwm_resume()
  pwm: rcar: remove legacy APIs
  pwm: rcar: add workaround to output "pseudo" low level

 drivers/pwm/pwm-rcar.c | 108 -
 1 file changed, 62 insertions(+), 46 deletions(-)

-- 
1.9.1

[PATCH 4/5] pwm: rcar: remove legacy APIs

[Yoshihiro Shimoda]

This patch removes legacy APIs. Since rcar_pwm_{en,dis}able()
functions are reused on "atomic" API, this patch changes
the arguments of these functions. No change in behavior.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/pwm/pwm-rcar.c | 45 -
 1 file changed, 4 insertions(+), 41 deletions(-)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index 1bcb662..e479b6a 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -146,40 +146,8 @@ static void rcar_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
pm_runtime_put(chip->dev);
 }
 
-static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
-  int duty_ns, int period_ns)
+static int rcar_pwm_enable(struct rcar_pwm_chip *rp)
 {
-   struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
-   int div, ret;
-
-   div = rcar_pwm_get_clock_division(rp, period_ns);
-   if (div < 0)
-   return div;
-
-   /*
-* Let the core driver set pwm->period if disabled and duty_ns == 0.
-* But, this driver should prevent to set the new duty_ns if current
-* duty_cycle is not set
-*/
-   if (!pwm_is_enabled(pwm) && !duty_ns && !pwm->state.duty_cycle)
-   return 0;
-
-   rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
-
-   rcar_pwm_calc_counter(rp, div, duty_ns, period_ns);
-   ret = rcar_pwm_set_counter(rp);
-   if (!ret)
-   rcar_pwm_set_clock_control(rp, div);
-
-   /* The SYNC should be set to 0 even if rcar_pwm_set_counter failed */
-   rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
-
-   return ret;
-}
-
-static int rcar_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
-{
-   struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
u32 value;
 
/* Don't enable the PWM device if CYC0 or PH0 is 0 */
@@ -193,10 +161,8 @@ static int rcar_pwm_enable(struct pwm_chip *chip, struct 
pwm_device *pwm)
return 0;
 }
 
-static void rcar_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+static void rcar_pwm_disable(struct rcar_pwm_chip *rp)
 {
-   struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
-
rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
 }
 
@@ -221,10 +187,10 @@ static int rcar_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
 
if (!ret && state->enabled)
-   ret = rcar_pwm_enable(chip, pwm);
+   ret = rcar_pwm_enable(rp);
 
if (!state->enabled) {
-   rcar_pwm_disable(chip, pwm);
+   rcar_pwm_disable(rp);
ret = 0;
}
 
@@ -234,9 +200,6 @@ static int rcar_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
 static const struct pwm_ops rcar_pwm_ops = {
.request = rcar_pwm_request,
.free = rcar_pwm_free,
-   .config = rcar_pwm_config,
-   .enable = rcar_pwm_enable,
-   .disable = rcar_pwm_disable,
.apply = rcar_pwm_apply,
.owner = THIS_MODULE,
 };
-- 
1.9.1

[PATCH 3/5] pwm: rcar: Use "atomic" API on rcar_pwm_resume()

[Yoshihiro Shimoda]

To remove legacy API related functions in the future, this patch
uses "atomic" related function instead. No change in behavior.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/pwm/pwm-rcar.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index a5ea0f3..1bcb662 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -319,18 +319,16 @@ static int rcar_pwm_suspend(struct device *dev)
 static int rcar_pwm_resume(struct device *dev)
 {
struct pwm_device *pwm = rcar_pwm_dev_to_pwm_dev(dev);
+   struct pwm_state state;
 
if (!test_bit(PWMF_REQUESTED, &pwm->flags))
return 0;
 
pm_runtime_get_sync(dev);
 
-   rcar_pwm_config(pwm->chip, pwm, pwm->state.duty_cycle,
-   pwm->state.period);
-   if (pwm_is_enabled(pwm))
-   rcar_pwm_enable(pwm->chip, pwm);
+   pwm_get_state(pwm, &state);
 
-   return 0;
+   return rcar_pwm_apply(pwm->chip, pwm, &state);
 }
 #endif /* CONFIG_PM_SLEEP */
 static SIMPLE_DEV_PM_OPS(rcar_pwm_pm_ops, rcar_pwm_suspend, rcar_pwm_resume);
-- 
1.9.1

[PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level

[Yoshihiro Shimoda]

This PWM Timer cannot output low because setting 0x000 is prohibited
on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding
the prohibited, this patch adds a workaround function to change
the value from 0 to 1 as pseudo low level.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/pwm/pwm-rcar.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index e479b6a..888cb37 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -166,6 +166,20 @@ static void rcar_pwm_disable(struct rcar_pwm_chip *rp)
rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
 }
 
+static void rcar_pwm_workaround_output_low(struct rcar_pwm_chip *rp)
+{
+   /*
+* This PWM Timer cannot output low because setting 0x000 is
+* prohibited on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding
+* the prohibited, this function changes the value from 0 to 1 as
+* pseudo low level.
+*
+* TODO: Add GPIO handling to output low level.
+*/
+   if ((rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
+   rp->pwmcnt |= 1;
+}
+
 static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
  struct pwm_state *state)
 {
@@ -179,6 +193,7 @@ static int rcar_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
 
rcar_pwm_calc_counter(rp, div, state->duty_cycle, state->period);
+   rcar_pwm_workaround_output_low(rp);
ret = rcar_pwm_set_counter(rp);
if (!ret)
rcar_pwm_set_clock_control(rp, div);
-- 
1.9.1

[PATCH 2/5] pwm: rcar: Add support "atomic" API

[Yoshihiro Shimoda]

This patch adds support for "atomic" API. Behavior is the same as
when using legacy APIs.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/pwm/pwm-rcar.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
index 9cf4567..a5ea0f3 100644
--- a/drivers/pwm/pwm-rcar.c
+++ b/drivers/pwm/pwm-rcar.c
@@ -200,12 +200,44 @@ static void rcar_pwm_disable(struct pwm_chip *chip, 
struct pwm_device *pwm)
rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
 }
 
+static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+   struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+   int div, ret;
+
+   div = rcar_pwm_get_clock_division(rp, state->period);
+   if (div < 0)
+   return div;
+
+   rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
+
+   rcar_pwm_calc_counter(rp, div, state->duty_cycle, state->period);
+   ret = rcar_pwm_set_counter(rp);
+   if (!ret)
+   rcar_pwm_set_clock_control(rp, div);
+
+   /* The SYNC should be set to 0 even if rcar_pwm_set_counter failed */
+   rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
+
+   if (!ret && state->enabled)
+   ret = rcar_pwm_enable(chip, pwm);
+
+   if (!state->enabled) {
+   rcar_pwm_disable(chip, pwm);
+   ret = 0;
+   }
+
+   return ret;
+}
+
 static const struct pwm_ops rcar_pwm_ops = {
.request = rcar_pwm_request,
.free = rcar_pwm_free,
.config = rcar_pwm_config,
.enable = rcar_pwm_enable,
.disable = rcar_pwm_disable,
+   .apply = rcar_pwm_apply,
.owner = THIS_MODULE,
 };
 
-- 
1.9.1

Re: [PATCH 1/5] pwm: rcar: add rcar_pwm_calc_counter() to calculate PWMCNT value only

[Uwe Kleine-König]

Hello,

On Fri, Dec 07, 2018 at 05:29:29PM +0900, Yoshihiro Shimoda wrote:
> -static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int 
> duty_ns,
> - int period_ns)
> +static void rcar_pwm_calc_counter(struct rcar_pwm_chip *rp, int div,
> +   int duty_ns, int period_ns)
>  {
>   unsigned long long one_cycle, tmp;  /* 0.01 nanoseconds */
>   unsigned long clk_rate = clk_get_rate(rp->clk);
> @@ -120,11 +121,17 @@ static int rcar_pwm_set_counter(struct rcar_pwm_chip 
> *rp, int div, int duty_ns,
>   do_div(tmp, one_cycle);
>   ph = tmp & RCAR_PWMCNT_PH0_MASK;
>  
> + rp->pwmcnt = cyc | ph;

Wouldn't it be more natural to let rcar_pwm_calc_counter return cyc | ph
and then ...

> +}
> +
> +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp)
> +{
>   /* Avoid prohibited setting */
> - if (cyc == 0 || ph == 0)
> + if ((rp->pwmcnt & RCAR_PWMCNT_CYC0_MASK) == 0 ||
> + (rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
>   return -EINVAL;
>  
> - rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> + rcar_pwm_write(rp, rp->pwmcnt, RCAR_PWMCNT);
>  
>   return 0;
>  }
> @@ -159,7 +166,8 @@ static int rcar_pwm_config(struct pwm_chip *chip, struct 
> pwm_device *pwm,
>  
>   rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
>  
> - ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> + rcar_pwm_calc_counter(rp, div, duty_ns, period_ns);
> + ret = rcar_pwm_set_counter(rp);
>   if (!ret)
>   rcar_pwm_set_clock_control(rp, div);

... pass this value to rcar_pwm_set_counter as u32. (Or pass div,
duty_ns and period_ns to rcar_pwm_set_counter and let this then call
rcar_pwm_calc_counter. Then you don't need a new member in the struct
rcar_pwm_chip.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH 2/5] pwm: rcar: Add support "atomic" API

[Uwe Kleine-König]

Hello,

On Fri, Dec 07, 2018 at 05:29:30PM +0900, Yoshihiro Shimoda wrote:
> This patch adds support for "atomic" API. Behavior is the same as
> when using legacy APIs.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/pwm/pwm-rcar.c | 32 
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> index 9cf4567..a5ea0f3 100644
> --- a/drivers/pwm/pwm-rcar.c
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -200,12 +200,44 @@ static void rcar_pwm_disable(struct pwm_chip *chip, 
> struct pwm_device *pwm)
>   rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
>  }
>  
> +static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +   struct pwm_state *state)
> +{
> + struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
> + int div, ret;
> +
> + div = rcar_pwm_get_clock_division(rp, state->period);
> + if (div < 0)
> + return div;
> +
> + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> +
> + rcar_pwm_calc_counter(rp, div, state->duty_cycle, state->period);
> + ret = rcar_pwm_set_counter(rp);
> + if (!ret)
> + rcar_pwm_set_clock_control(rp, div);
> +
> + /* The SYNC should be set to 0 even if rcar_pwm_set_counter failed */
> + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
> +
> + if (!ret && state->enabled)
> + ret = rcar_pwm_enable(chip, pwm);
> +
> + if (!state->enabled) {
> + rcar_pwm_disable(chip, pwm);
> + ret = 0;
> + }
> +
> + return ret;

state->polarity isn't used here which is a bug I think.

Is the documentation for this hardware publically available?

If the pwm runs at say 30% duty cycle and then pwm_apply is called with

.period = 1000,
.duty_cycle = 600,
.enabled = false,

can it happen that for some time a duty cycle of 60% is provided? If so,
that's a bug.

Out of interest: How does the output behave if you disable the hardware?
Does it give a 0, or high-z?

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH 5/5] pwm: rcar: add workaround to output "pseudo" low level

[Uwe Kleine-König]

On Fri, Dec 07, 2018 at 05:29:33PM +0900, Yoshihiro Shimoda wrote:
> This PWM Timer cannot output low because setting 0x000 is prohibited
> on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding
> the prohibited, this patch adds a workaround function to change
> the value from 0 to 1 as pseudo low level.
> 
> Signed-off-by: Yoshihiro Shimoda 
> ---
>  drivers/pwm/pwm-rcar.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> index e479b6a..888cb37 100644
> --- a/drivers/pwm/pwm-rcar.c
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -166,6 +166,20 @@ static void rcar_pwm_disable(struct rcar_pwm_chip *rp)
>   rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
>  }
>  
> +static void rcar_pwm_workaround_output_low(struct rcar_pwm_chip *rp)
> +{
> + /*
> +  * This PWM Timer cannot output low because setting 0x000 is
> +  * prohibited on PWMCNT.PH0 (High-Level Period) bitfields. So, avoiding
> +  * the prohibited, this function changes the value from 0 to 1 as
> +  * pseudo low level.
> +  *
> +  * TODO: Add GPIO handling to output low level.
> +  */
> + if ((rp->pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
> + rp->pwmcnt |= 1;

In my eyes this is too broken to do. Not sure I have the complete
picture, but given a small period (say 2) this 1 cycle might result in
50 % duty cycle. Depending on how the hardware behaves if you disable
it, better do this instead.

Are you aware of the series adding such gpio support to the imx driver?

@Thierry: So there are three drivers now that could benefit for a
generic approach.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc

[Biju Das]

Hi Alexandre,

Thanks for the feedback.

> Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc
>
> On 06/12/2018 15:49:57+, Biju Das wrote:
> > Hi Geert,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > pcf85263 rtc
> > >
> > > Hi Biju,
> > >
> > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das 
> wrote:
> > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > > > pcf85263 rtc CC nvmem maintainer
> > > > >
> > > > > On Thu, Dec 6, 2018 at 10:04 AM Biju Das
> > > > > 
> > > wrote:
> > > > > > Add support for NXP pcf85263 real-time clock. pcf85263 rtc is
> > > > > > compatible with pcf85363,except that pcf85363 has additional
> > > > > > 64 bytes of
> > > > > RAM.
> > >
> > > > > > --- a/drivers/rtc/rtc-pcf85363.c
> > > > > > +++ b/drivers/rtc/rtc-pcf85363.c
> > > > > > @@ -120,6 +120,11 @@ struct pcf85363 {
> > > > > > struct regmap   *regmap;
> > > > > >  };
> > > > > >
> > > > > > +struct pcf85x63_config {
> > > > > > +   struct regmap_config regmap;
> > > > > > +   unsigned int num_nvram; };
> > > > > > +
> > > > > >  static int pcf85363_rtc_read_time(struct device *dev, struct
> > > > > > rtc_time
> > > > > > *tm)  {
> > > > > > struct pcf85363 *pcf85363 = dev_get_drvdata(dev); @@
> > > > > > -311,25
> > > > > > +316,68 @@ static int pcf85363_nvram_write(void *priv,
> > > > > > +unsigned int
> > > > > offset, void *val,
> > > > > >  val, bytes);  }
> > > > > >
> > > > > > -static const struct regmap_config regmap_config = {
> > > > > > -   .reg_bits = 8,
> > > > > > -   .val_bits = 8,
> > > > > > -   .max_register = 0x7f,
> > > > > > +static int pcf85x63_nvram_read(void *priv, unsigned int
> > > > > > +offset, void
> > > *val,
> > > > > > +  size_t bytes)
> > > > >
> > > > > Given bytes should be 1, val should be a pointer to a single byte...
> > > > > What if bytes == 0?
> > > >
> > > > I doubt we get "bytes==0" because of the checks in "
> > > drivers/nvmem/core.c"
> > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write".
> > >
> > > Depends. There are other functions calling nvmem_reg_{read,write}(),
> e.g.
> > > nvmem_device_{read,write}().
> >
> > OK. In that case, I will return (-EINVAL)  for "bytes !=1"
> >
>
> I think it is probably better to ensure the nvmem core never passes an invalid
> number of bytes. All the ther RTC drivers make that assumption.

In that case, I will do following checks in nvmem_device_{read,write}() before 
calling nvmem_reg_{read,write}(),

nvmem_device_read

/* Stop the user from reading */
if (offset  >= nvmem->size)
return 0;

if (bytes == 0)
return -EINVAL;

if (offset + bytes > nvmem->size)
bytes = nvmem->size - offset;

nvmem_device_write

/* Stop the user from writing */
if (offset  >= nvmem->size)
return -EFBIG;

if (bytes == 0)
return -EINVAL;

if (offset + bytes > nvmem->size)
bytes = nvmem->size - offset;

regards,
Biju





[https://www2.renesas.eu/media/email/unicef.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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

Re: [PATCH 2/4] mtd: spi-nor: Add support for is25lp016d

[Tudor.Ambarus]

On 11/01/2018 02:35 PM, Fabrizio Castro wrote:
> The is25lp016d is found on the iwg23s from iWave, therefore
> add driver support for it so that we can upstream board support.
> 
> Signed-off-by: Fabrizio Castro 

Reviewed-by: Tudor Ambarus 

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 9407ca5..85d869b 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1352,6 +1352,8 @@ static const struct flash_info spi_nor_ids[] = {
>   { "is25cd512",  INFO(0x7f9d20, 0, 32 * 1024,   2, SECT_4K) },
>   { "is25lq040b", INFO(0x9d4013, 0, 64 * 1024,   8,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { "is25lp016d", INFO(0x9d6015, 0, 64 * 1024,  32,
> + SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { "is25lp080d", INFO(0x9d6014, 0, 64 * 1024,  16,
>   SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { "is25lp128",  INFO(0x9d6018, 0, 64 * 1024, 256,
> 

Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 7, 2018 at 10:34 AM Biju Das  wrote:
> > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc
> >
> > On 06/12/2018 15:49:57+, Biju Das wrote:

> > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > > pcf85263 rtc

> > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das 
> > wrote:
> > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > > > > pcf85263 rtc CC nvmem maintainer
> > > > > > Given bytes should be 1, val should be a pointer to a single byte...
> > > > > > What if bytes == 0?
> > > > >
> > > > > I doubt we get "bytes==0" because of the checks in "
> > > > drivers/nvmem/core.c"
> > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write".
> > > >
> > > > Depends. There are other functions calling nvmem_reg_{read,write}(),
> > e.g.
> > > > nvmem_device_{read,write}().
> > >
> > > OK. In that case, I will return (-EINVAL)  for "bytes !=1"
> >
> > I think it is probably better to ensure the nvmem core never passes an 
> > invalid
> > number of bytes. All the ther RTC drivers make that assumption.
>
> In that case, I will do following checks in nvmem_device_{read,write}() 
> before calling nvmem_reg_{read,write}(),
>
> nvmem_device_read
>
> /* Stop the user from reading */
> if (offset  >= nvmem->size)
> return 0;
>
> if (bytes == 0)
> return -EINVAL;

Why not 0?

>
> if (offset + bytes > nvmem->size)

This might overflow, please use check_add_overflow().

> bytes = nvmem->size - offset;
>
> nvmem_device_write
>
> /* Stop the user from writing */
> if (offset  >= nvmem->size)
> return -EFBIG;

ENOSPC?

+ same comments as for read.

Oh, and offset is unsigned int instead of loff_t.
Nobody's envisioning nvmem devices larger than 4 GiB?

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 2/5] pwm: rcar: Add support "atomic" API

[Geert Uytterhoeven]

Hi Uwe,

On Fri, Dec 7, 2018 at 10:08 AM Uwe Kleine-König
 wrote:
> On Fri, Dec 07, 2018 at 05:29:30PM +0900, Yoshihiro Shimoda wrote:
> > This patch adds support for "atomic" API. Behavior is the same as
> > when using legacy APIs.
> >
> > Signed-off-by: Yoshihiro Shimoda 
> > ---
> >  drivers/pwm/pwm-rcar.c | 32 
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> > index 9cf4567..a5ea0f3 100644
> > --- a/drivers/pwm/pwm-rcar.c
> > +++ b/drivers/pwm/pwm-rcar.c
> > @@ -200,12 +200,44 @@ static void rcar_pwm_disable(struct pwm_chip *chip, 
> > struct pwm_device *pwm)
> >   rcar_pwm_update(rp, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
> >  }
> >
> > +static int rcar_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > +   struct pwm_state *state)
> > +{
> > + struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
> > + int div, ret;
> > +
> > + div = rcar_pwm_get_clock_division(rp, state->period);
> > + if (div < 0)
> > + return div;
> > +
> > + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> > +
> > + rcar_pwm_calc_counter(rp, div, state->duty_cycle, state->period);
> > + ret = rcar_pwm_set_counter(rp);
> > + if (!ret)
> > + rcar_pwm_set_clock_control(rp, div);
> > +
> > + /* The SYNC should be set to 0 even if rcar_pwm_set_counter failed */
> > + rcar_pwm_update(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
> > +
> > + if (!ret && state->enabled)
> > + ret = rcar_pwm_enable(chip, pwm);
> > +
> > + if (!state->enabled) {
> > + rcar_pwm_disable(chip, pwm);
> > + ret = 0;
> > + }
> > +
> > + return ret;
>
> state->polarity isn't used here which is a bug I think.
>
> Is the documentation for this hardware publically available?

Please check Section 59 of the "User's Manual: Hardware" at
https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rzg/rzg1m.html

Thanks!

Gr{oetje,eeting}s,

Geert

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

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

RE: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc
>
> Hi Biju,
>
> On Fri, Dec 7, 2018 at 10:34 AM Biju Das  wrote:
> > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > pcf85263 rtc
> > >
> > > On 06/12/2018 15:49:57+, Biju Das wrote:
>
> > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > > > pcf85263 rtc
>
> > > > > On Thu, Dec 6, 2018 at 4:24 PM Biju Das
> > > > > 
> > > wrote:
> > > > > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for
> > > > > > > NXP
> > > > > > > pcf85263 rtc CC nvmem maintainer Given bytes should be 1,
> > > > > > > val should be a pointer to a single byte...
> > > > > > > What if bytes == 0?
> > > > > >
> > > > > > I doubt we get "bytes==0" because of the checks in "
> > > > > drivers/nvmem/core.c"
> > > > > > Function " bin_attr_nvmem_read/ bin_attr_nvmem_write".
> > > > >
> > > > > Depends. There are other functions calling
> > > > > nvmem_reg_{read,write}(),
> > > e.g.
> > > > > nvmem_device_{read,write}().
> > > >
> > > > OK. In that case, I will return (-EINVAL)  for "bytes !=1"
> > >
> > > I think it is probably better to ensure the nvmem core never passes
> > > an invalid number of bytes. All the ther RTC drivers make that
> assumption.
> >
> > In that case, I will do following checks in
> > nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(),
> >
> > nvmem_device_read
> >
> > /* Stop the user from reading */
> > if (offset  >= nvmem->size)
> > return 0;
> >
> > if (bytes == 0)
> > return -EINVAL;
>
> Why not 0?

Ok. Will merge with above check.

if ((offset  >= nvmem->size)  && (bytes == 0))
return 0;

> >
> > if (offset + bytes > nvmem->size)
>
> This might overflow, please use check_add_overflow().

Will use check_add_overflow() and then the  result is compared with 
nvmem->size, if the check operation is successful.

> > bytes = nvmem->size - offset;
> >
> > nvmem_device_write
> >
> > /* Stop the user from writing */
> > if (offset  >= nvmem->size)
> > return -EFBIG;
>
> ENOSPC?

OK, Will change it to ENOSPC.

> + same comments as for read.
>
> Oh, and offset is unsigned int instead of loff_t.
> Nobody's envisioning nvmem devices larger than 4 GiB?

Regards,
Biju


[https://www2.renesas.eu/media/email/unicef.jpg]

This Christmas, instead of sending out cards, Renesas Electronics Europe have 
decided to support Unicef with a donation. For further details click 
here to find out about the valuable work they do, 
helping children all over the world.
We would like to take this opportunity to wish you a Merry Christmas and a 
prosperous New Year.



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 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc

[Geert Uytterhoeven]

Hi Biju,

On Fri, Dec 7, 2018 at 11:16 AM Biju Das  wrote:
> > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP pcf85263 rtc
> > On Fri, Dec 7, 2018 at 10:34 AM Biju Das  wrote:
> > > > Subject: Re: [PATCH v3 2/4] rtc: pcf85363: Add support for NXP
> > > > pcf85263 rtc
> > > > I think it is probably better to ensure the nvmem core never passes
> > > > an invalid number of bytes. All the ther RTC drivers make that
> > assumption.
> > >
> > > In that case, I will do following checks in
> > > nvmem_device_{read,write}() before calling nvmem_reg_{read,write}(),
> > >
> > > nvmem_device_read
> > >
> > > /* Stop the user from reading */
> > > if (offset  >= nvmem->size)
> > > return 0;
> > >
> > > if (bytes == 0)
> > > return -EINVAL;
> >
> > Why not 0?
>
> Ok. Will merge with above check.
>
> if ((offset  >= nvmem->size)  && (bytes == 0))

"||" ;-)

> return 0;

Gr{oetje,eeting}s,

Geert

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

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

Re: [PATCH 2/5] pwm: rcar: Add support "atomic" API

[Uwe Kleine-König]

Hello,

On Fri, Dec 07, 2018 at 10:57:48AM +0100, Geert Uytterhoeven wrote:
> > Is the documentation for this hardware publically available?
> 
> Please check Section 59 of the "User's Manual: Hardware" at
> https://www.renesas.com/eu/en/products/microcontrollers-microprocessors/rz/rzg/rzg1m.html

Thanks.

So the ugly things here are:

 a) if the pwm is stopped (PWMCR.ENO = 0) the output is set to high after
completion of the currently running period.
 b) setting a duty_cycle of 0 is forbidden

both are bad. A workaround for both is implementation of something
similar to the switch to a gpio suggested by Michal for imx. But this
cannot be done reliably because the current period's end isn't
observable.

Alternatively a confirmation from the Renesas engineers that PWMCNT.PHO
can be set to 0 with the intended effect despite being forbidden in the
reference manual would be great. Did someone with access to such
hardware test what happens if the PHO field is set to 0? Maybe the
forbidden value is just a wrong copy&paste from the CYCO field?

I think it would be a good idea to add the link to the documentation
into a comment at the top of the driver.

@Thierry: Given that nobody seems to have an overview about the features
and ugly implementation details of all the PWMs, what about documenting
them in the driver files in a greppable way. For the rcar driver
something like:

 - duty-counter-bits: 10
 - period-counter-bits: 10
 - hardware-polarity-support: false
 - uglyness:
   - OUTPUT-ACTIVE-ON-DISABLE
   - NO-ZERO-DUTY-CYCLE

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

[PATCH v4 2/5] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock

[Biju Das]

The pcf85263 RTC is compatible with the pcf85363 RTC.

The difference between the pcf85263 and pcf85363 RTC is that the latter has
64 bytes more RAM. This renders them incompatible from a DT point of view.

Signed-off-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
V1-->V2
* Incorporated Simon's review comment.
V2-->V3
* No Change.
V3-->V4
* No Change.
---
 Documentation/devicetree/bindings/rtc/pcf85363.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/pcf85363.txt 
b/Documentation/devicetree/bindings/rtc/pcf85363.txt
index 76fdabc..94adc1c 100644
--- a/Documentation/devicetree/bindings/rtc/pcf85363.txt
+++ b/Documentation/devicetree/bindings/rtc/pcf85363.txt
@@ -1,8 +1,8 @@
-NXP PCF85363 Real Time Clock
+NXP PCF85263/PCF85363 Real Time Clock
 
 
 Required properties:
-- compatible: Should contain "nxp,pcf85363".
+- compatible: Should contain "nxp,pcf85263" or "nxp,pcf85363".
 - reg: I2C address for chip.
 
 Optional properties:
-- 
2.7.4

[PATCH v4 0/5] Add NXP pcf85263 real-time clock support

[Biju Das]

This patch set aims to add support for NXP pcf85263 real-time clock.
pcf85263 rtc is compatible with pcf85363 rtc except that pcf85363 has
64 bytes additional RAM.

1 byte of nvmem is supported in pcf85263 and is exposed through sysfs.

The details of pcf85363 and pcf85263 can be found in the below data sheets.

https://www.nxp.com/docs/en/data-sheet/PCF85363A.pdf

https://www.nxp.com/docs/en/data-sheet/PCF85263A.pdf

This patch is tested against linux-next.

V1-->V2
   * Incorporated simon's review comment for binding patch.
   * Incorporated Geert and Alexandre's review comments.
V2-->V3
   * Incorporated Geert's review comments.
V3-->V4
   * Incorporated Geert and Alexandre's review comments.

Biju Das (5):
  nvmem: check invalid number of bytes in nvmem_device_{read,write}
  dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock
  rtc: pcf85363: Add support for NXP pcf85263 rtc
  ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig
  ARM: dts: iwg23s-sbc: Enable RTC

 Documentation/devicetree/bindings/rtc/pcf85363.txt |  4 +-
 arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts  | 18 +
 arch/arm/configs/shmobile_defconfig|  1 +
 drivers/nvmem/core.c   | 26 +-
 drivers/rtc/rtc-pcf85363.c | 94 +-
 5 files changed, 122 insertions(+), 21 deletions(-)

-- 
2.7.4

[PATCH v4 1/5] nvmem: check invalid number of bytes in nvmem_device_{read,write}

[Biju Das]

Add check in nvmem_device_{read,write}()to ensure that nvmem core never
passes an invalid number of bytes.

Signed-off-by: Biju Das 
---
V3-->V4
* New patch.
---
 drivers/nvmem/core.c | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index d9fd110..db7de33 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1433,10 +1433,21 @@ int nvmem_device_read(struct nvmem_device *nvmem,
  size_t bytes, void *buf)
 {
int rc;
+   size_t new_bytes;
 
if (!nvmem)
return -EINVAL;
 
+   /* Stop the user from reading */
+   if ((offset >= nvmem->size) || (bytes == 0))
+   return 0;
+
+   if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
+   return -EOVERFLOW;
+
+   if (new_bytes > nvmem->size)
+   bytes = nvmem->size - offset;
+
rc = nvmem_reg_read(nvmem, offset, buf, bytes);
 
if (rc)
@@ -1461,16 +1472,29 @@ int nvmem_device_write(struct nvmem_device *nvmem,
   size_t bytes, void *buf)
 {
int rc;
+   size_t new_bytes;
 
if (!nvmem)
return -EINVAL;
 
+   /* Stop the user from writing */
+   if (offset >= nvmem->size)
+   return -ENOSPC;
+
+   if (bytes == 0)
+   return 0;
+
+   if (unlikely(check_add_overflow(bytes, offset, &new_bytes)))
+   return -EOVERFLOW;
+
+   if (new_bytes > nvmem->size)
+   bytes = nvmem->size - offset;
+
rc = nvmem_reg_write(nvmem, offset, buf, bytes);
 
if (rc)
return rc;
 
-
return bytes;
 }
 EXPORT_SYMBOL_GPL(nvmem_device_write);
-- 
2.7.4

[PATCH v4 5/5] ARM: dts: iwg23s-sbc: Enable RTC

[Biju Das]

Enable NXP pcf85263 real time clock for the iWave SBC based on RZ/G1C.

Signed-off-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
V1-->V2
* No change
V2-->V3
* No change
V3-->V4
* No change
---
 arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts 
b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts
index 40b7f98..77d1824 100644
--- a/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts
+++ b/arch/arm/boot/dts/r8a77470-iwg23s-sbc.dts
@@ -84,12 +84,30 @@
clock-frequency = <2000>;
 };
 
+&i2c3 {
+   pinctrl-0 = <&i2c3_pins>;
+   pinctrl-names = "default";
+
+   status = "okay";
+   clock-frequency = <40>;
+
+   rtc@51 {
+   compatible = "nxp,pcf85263";
+   reg = <0x51>;
+   };
+};
+
 &pfc {
avb_pins: avb {
groups = "avb_mdio", "avb_gmii_tx_rx";
function = "avb";
};
 
+   i2c3_pins: i2c3 {
+   groups = "i2c3_c";
+   function = "i2c3";
+   };
+
mmc_pins_uhs: mmc_uhs {
groups = "mmc_data8", "mmc_ctrl";
function = "mmc";
-- 
2.7.4

[PATCH v4 3/5] rtc: pcf85363: Add support for NXP pcf85263 rtc

[Biju Das]

Add support for NXP pcf85263 real-time clock. pcf85263 rtc is compatible
with pcf85363,except that pcf85363 has additional 64 bytes of RAM.

1 byte of nvmem is supported and exposed in sysfs (# is the instance
number,starting with 0): /sys/bus/nvmem/devices/pcf85x63-#/nvmem

Signed-off-by: Biju Das 
---
V1-->V2
* Incorporated Alexandre and Geert's review comment.
V2-->V3
* Incorporated Geert's review comment.
V3-->V4
* Incorporated Geert's and Alexandre's review comment.
---
 drivers/rtc/rtc-pcf85363.c | 94 +-
 1 file changed, 76 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index c04a1ed..a398807 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -120,6 +120,11 @@ struct pcf85363 {
struct regmap   *regmap;
 };
 
+struct pcf85x63_config {
+   struct regmap_config regmap;
+   unsigned int num_nvram;
+};
+
 static int pcf85363_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
@@ -311,25 +316,75 @@ static int pcf85363_nvram_write(void *priv, unsigned int 
offset, void *val,
 val, bytes);
 }
 
-static const struct regmap_config regmap_config = {
-   .reg_bits = 8,
-   .val_bits = 8,
-   .max_register = 0x7f,
+static int pcf85x63_nvram_read(void *priv, unsigned int offset, void *val,
+  size_t bytes)
+{
+   struct pcf85363 *pcf85363 = priv;
+   unsigned int tmp_val;
+   int ret;
+
+   ret = regmap_read(pcf85363->regmap, CTRL_RAMBYTE, &tmp_val);
+   (*(unsigned char *) val) = (unsigned char) tmp_val;
+
+   return ret;
+}
+
+static int pcf85x63_nvram_write(void *priv, unsigned int offset, void *val,
+   size_t bytes)
+{
+   struct pcf85363 *pcf85363 = priv;
+   unsigned char tmp_val;
+
+   tmp_val = *((unsigned char *)val);
+   return regmap_write(pcf85363->regmap, CTRL_RAMBYTE,
+   (unsigned int)tmp_val);
+}
+
+static const struct pcf85x63_config pcf_85263_config = {
+   .regmap = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = 0x2f,
+   },
+   .num_nvram = 1
+};
+
+static const struct pcf85x63_config pcf_85363_config = {
+   .regmap = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = 0x7f,
+   },
+   .num_nvram = 2
 };
 
 static int pcf85363_probe(struct i2c_client *client,
  const struct i2c_device_id *id)
 {
struct pcf85363 *pcf85363;
-   struct nvmem_config nvmem_cfg = {
-   .name = "pcf85363-",
-   .word_size = 1,
-   .stride = 1,
-   .size = NVRAM_SIZE,
-   .reg_read = pcf85363_nvram_read,
-   .reg_write = pcf85363_nvram_write,
+   const struct pcf85x63_config *config = &pcf_85363_config;
+   const void *data = of_device_get_match_data(&client->dev);
+   static struct nvmem_config nvmem_cfg[] = {
+   {
+   .name = "pcf85x63-",
+   .word_size = 1,
+   .stride = 1,
+   .size = 1,
+   .reg_read = pcf85x63_nvram_read,
+   .reg_write = pcf85x63_nvram_write,
+   }, {
+   .name = "pcf85363-",
+   .word_size = 1,
+   .stride = 1,
+   .size = NVRAM_SIZE,
+   .reg_read = pcf85363_nvram_read,
+   .reg_write = pcf85363_nvram_write,
+   },
};
-   int ret;
+   int ret, i;
+
+   if (data)
+   config = data;
 
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
return -ENODEV;
@@ -339,7 +394,7 @@ static int pcf85363_probe(struct i2c_client *client,
if (!pcf85363)
return -ENOMEM;
 
-   pcf85363->regmap = devm_regmap_init_i2c(client, ®map_config);
+   pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
if (IS_ERR(pcf85363->regmap)) {
dev_err(&client->dev, "regmap allocation failed\n");
return PTR_ERR(pcf85363->regmap);
@@ -370,15 +425,18 @@ static int pcf85363_probe(struct i2c_client *client,
 
ret = rtc_register_device(pcf85363->rtc);
 
-   nvmem_cfg.priv = pcf85363;
-   rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg);
+   for (i = 0; i < config->num_nvram; i++) {
+   nvmem_cfg[i].priv = pcf85363;
+   rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
+   }
 
return ret;
 }
 
 static const struct of_device_id dev_ids[] = {
-   { .compatible = "nxp,pcf85363" },
-   {}
+   { .compatible = "nxp,pcf85263", .data 

[PATCH v4 4/5] ARM: shmobile: Enable NXP pcf85363 rtc in shmobile_defconfig

[Biju Das]

The iWave RZ/G1C SBC supports RTC (NXP pcf85263). To increase hardware
support enable the driver in the shmobile_defconfig multiplatform
configuration.

Signed-off-by: Biju Das 
Reviewed-by: Geert Uytterhoeven 
---
V1-->V2
* No change.
V2-->V3
* No change.
V3-->V4
* No Change.
---
 arch/arm/configs/shmobile_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/shmobile_defconfig 
b/arch/arm/configs/shmobile_defconfig
index 9e5a5ad..fdac4e4 100644
--- a/arch/arm/configs/shmobile_defconfig
+++ b/arch/arm/configs/shmobile_defconfig
@@ -177,6 +177,7 @@ CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
 CONFIG_RTC_CLASS=y
 CONFIG_RTC_DRV_RS5C372=y
+CONFIG_RTC_DRV_PCF85363=y
 CONFIG_RTC_DRV_BQ32K=y
 CONFIG_RTC_DRV_S35390A=y
 CONFIG_RTC_DRV_RX8581=y
-- 
2.7.4

[git pull] pinctrl: sh-pfc: Updates for v4.21

[Geert Uytterhoeven]

Hi Linus,

The following changes since commit b59d0e782706785b7042539e820e95df3be4d04c:

  pinctrl: Add RZ/A2 pin and gpio controller (2018-11-23 09:30:27 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
tags/sh-pfc-for-v4.21-tag2

for you to fetch changes up to 3f3327dbc5596076f94695d8d4cc66da3d5027fb:

  pinctrl: rzn1: Fix of_get_child_count() error check (2018-12-04 10:33:49 
+0100)


pinctrl: sh-pfc: Updates for v4.21 (take two)

  - Two small fixes for RZ/N1.

Thanks for pulling!


Phil Edworthy (2):
  pinctrl: rzn1: Fix check for used MDIO bus
  pinctrl: rzn1: Fix of_get_child_count() error check

 drivers/pinctrl/pinctrl-rzn1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[git pull] pinctrl: sh-pfc: Updates for v4.21 (take two)

[Geert Uytterhoeven]

Hi Linus,

The following changes since commit b59d0e782706785b7042539e820e95df3be4d04c:

  pinctrl: Add RZ/A2 pin and gpio controller (2018-11-23 09:30:27 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
tags/sh-pfc-for-v4.21-tag2

for you to fetch changes up to 3f3327dbc5596076f94695d8d4cc66da3d5027fb:

  pinctrl: rzn1: Fix of_get_child_count() error check (2018-12-04 10:33:49 
+0100)


pinctrl: sh-pfc: Updates for v4.21 (take two)

  - Two small fixes for RZ/N1.

Thanks for pulling!


Phil Edworthy (2):
  pinctrl: rzn1: Fix check for used MDIO bus
  pinctrl: rzn1: Fix of_get_child_count() error check

 drivers/pinctrl/pinctrl-rzn1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[git pull] clk: renesas: Updates for v4.21 (take two)

[Geert Uytterhoeven]

Hi Mike, Stephen,

The following changes since commit eb38c119dd91c61de26f67050671a84064554f7d:

  clk: renesas: r7s9210: Add USB clocks (2018-11-13 09:58:51 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
tags/clk-renesas-for-v4.21-tag2

for you to fetch changes up to 36c4da4f552a126bb29a95dc5c9608795491e32a:

  clk: renesas: rcar-gen3: Add HS400 quirk for SD clock (2018-12-07 11:45:06 
+0100)


clk: renesas: Updates for v4.21 (take two)

  - Add support for CPEX (timer) clocks on various R-Car Gen3 and RZ/G2
SoCs,
  - Add support for SDHI HS400 clocks on early revisions of R-Car H3 and
M3-W,
  - Miscellaneous fixes based on the Hardware Manual Errata.

Thanks for pulling!


Geert Uytterhoeven (12):
  dt-bindings: clock: r8a7795: Remove CSIREF clock
  dt-bindings: clock: r8a7796: Remove CSIREF clock
  clk: renesas: r8a774a1: Add CPEX clock
  clk: renesas: r8a7795: Add CPEX clock
  clk: renesas: r8a7796: Add CPEX clock
  clk: renesas: r8a77965: Add CPEX clock
  clk: renesas: r8a77970: Add CPEX clock
  clk: renesas: r8a77995: Correct parent clock of DU
  clk: renesas: r8a77995: Remove non-existent VIN5-7 module clocks
  clk: renesas: r8a77995: Remove non-existent SSP clocks
  clk: renesas: r8a77995: Add missing CPEX clock
  clk: renesas: r8a77995: Simplify PLL3 multiplier/divider

Niklas Söderlund (3):
  clk: renesas: rcar-gen3: Set state when registering SD clocks
  clk: renesas: rcar-gen3: Add documentation for SD clocks
  clk: renesas: rcar-gen3: Add HS400 quirk for SD clock

Takeshi Kihara (1):
  clk: renesas: r8a77990: Correct parent clock of DU

 drivers/clk/renesas/r8a774a1-cpg-mssr.c   |  1 +
 drivers/clk/renesas/r8a7795-cpg-mssr.c|  1 +
 drivers/clk/renesas/r8a7796-cpg-mssr.c|  1 +
 drivers/clk/renesas/r8a77965-cpg-mssr.c   |  1 +
 drivers/clk/renesas/r8a77970-cpg-mssr.c   |  1 +
 drivers/clk/renesas/r8a77990-cpg-mssr.c   |  4 +-
 drivers/clk/renesas/r8a77995-cpg-mssr.c   | 15 +++-
 drivers/clk/renesas/rcar-gen3-cpg.c   | 55 ---
 include/dt-bindings/clock/r8a7795-cpg-mssr.h  |  2 +-
 include/dt-bindings/clock/r8a7796-cpg-mssr.h  |  2 +-
 include/dt-bindings/clock/r8a77995-cpg-mssr.h |  5 ++-
 11 files changed, 51 insertions(+), 37 deletions(-)

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: [git pull] pinctrl: sh-pfc: Updates for v4.21 (take two)

[Linus Walleij]

On Fri, Dec 7, 2018 at 1:21 PM Geert Uytterhoeven
 wrote:

> The following changes since commit b59d0e782706785b7042539e820e95df3be4d04c:
>
>   pinctrl: Add RZ/A2 pin and gpio controller (2018-11-23 09:30:27 +0100)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
> tags/sh-pfc-for-v4.21-tag2
>
> for you to fetch changes up to 3f3327dbc5596076f94695d8d4cc66da3d5027fb:
>
>   pinctrl: rzn1: Fix of_get_child_count() error check (2018-12-04 10:33:49 
> +0100)

Merged this "take two" version into my devel branch!

Thanks!
Linus Walleij

[PATCH] pinctrl: sh-pfc: r8a77990: Add support for pull-up only pins

[Geert Uytterhoeven]

The R-Car Gen3 HardWare Manual Errata for Rev. 1.00 (Jul 2, 2018) states
that the USB30_OVC pin supports pull-up only.  It has a bit assigned in
the pull-enable register (PUEN5), but not in the pull-up/down control
register (PUD5).

Add a check for this, to prevent configuring a prohibited setting.

Reported-by: Yoshihiro Shimoda 
Fixes: 83f6941a42a5e773 ("pinctrl: sh-pfc: r8a77990: Add bias pinconf support")
Signed-off-by: Geert Uytterhoeven 
---
To be queued in sh-pfc-for-v4.22.

 drivers/pinctrl/sh-pfc/pfc-r8a77990.c | 23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c 
b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
index 8c06d72753890e32..784d4400c3e0e7ea 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a77990.c
@@ -4914,6 +4914,17 @@ static const struct pinmux_bias_reg pinmux_bias_regs[] = 
{
{ /* sentinel */ },
 };
 
+static bool pin_has_pud(unsigned int pin)
+{
+   /* Some pins are pull-up only */
+   switch (pin) {
+   case RCAR_GP_PIN(6, 9): /* USB30_OVC  */
+   return false;
+   }
+
+   return true;
+}
+
 static unsigned int r8a77990_pinmux_get_bias(struct sh_pfc *pfc,
 unsigned int pin)
 {
@@ -4926,7 +4937,7 @@ static unsigned int r8a77990_pinmux_get_bias(struct 
sh_pfc *pfc,
 
if (!(sh_pfc_read(pfc, reg->puen) & BIT(bit)))
return PIN_CONFIG_BIAS_DISABLE;
-   else if (sh_pfc_read(pfc, reg->pud) & BIT(bit))
+   else if (!pin_has_pud(pin) || (sh_pfc_read(pfc, reg->pud) & BIT(bit)))
return PIN_CONFIG_BIAS_PULL_UP;
else
return PIN_CONFIG_BIAS_PULL_DOWN;
@@ -4947,11 +4958,13 @@ static void r8a77990_pinmux_set_bias(struct sh_pfc 
*pfc, unsigned int pin,
if (bias != PIN_CONFIG_BIAS_DISABLE)
enable |= BIT(bit);
 
-   updown = sh_pfc_read(pfc, reg->pud) & ~BIT(bit);
-   if (bias == PIN_CONFIG_BIAS_PULL_UP)
-   updown |= BIT(bit);
+   if (pin_has_pud(pin)) {
+   updown = sh_pfc_read(pfc, reg->pud) & ~BIT(bit);
+   if (bias == PIN_CONFIG_BIAS_PULL_UP)
+   updown |= BIT(bit);
 
-   sh_pfc_write(pfc, reg->pud, updown);
+   sh_pfc_write(pfc, reg->pud, updown);
+   }
sh_pfc_write(pfc, reg->puen, enable);
 }
 
-- 
2.17.1

Re: [PATCH 1/2] ARM: dts: stout: Convert to new LVDS DT bindings

[Marek Vasut]

On 12/05/2018 06:41 AM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Wednesday, 5 December 2018 03:10:18 EET Marek Vasut wrote:
>> On 12/03/2018 11:48 PM, Laurent Pinchart wrote:
>>> On Tuesday, 4 December 2018 00:24:32 EET Marek Vasut wrote:
 On 12/03/2018 10:48 PM, Laurent Pinchart wrote:
> On Monday, 3 December 2018 17:12:41 EET Geert Uytterhoeven wrote:
>> As of commit 6d2ca85279becdff ("dt-bindings: display: renesas:
>> Deprecate LVDS support in the DU bindings"), the internal LVDS encoder
>> has DT bindings separate from the DU.  The Lager device tree was ported
>> over to the new model, but the Stout device tree was forgotten.
>>
>> Fixes: 15a1ff30d8f9bd83 ("ARM: dts: r8a7790: Convert to new LVDS DT
>> bindings") Signed-off-by: Geert Uytterhoeven 
>> ---
>> Compile-tested only.
>
> I can't test the patch either but it looks fine to me.
>
> Reviewed-by: Laurent Pinchart 
>
> I assume you will send this directly to Simon, so I don't plan to take
> the patch in my tree.

 I have a Stout if you need me to test something.
>>>
>>> Could you test HDMI output ? We just need to ensure that everything is
>>> probed correctly, so display anything on the HDMI output will do.
>>
>> Sure, can you push me a branch somewhere, so I don't have to hunt down
>> patches ?
> 
> Only this patch should be needed. You can get it from https://
> patchwork.kernel.org/patch/10709781/ and apply it on top of v4.19. Please 
> don't use v4.20-rc is there's one missing regression fix there (it has been 
> merged in the media tree and should make it to the final release).

On R8A7790 H2 ES3.0 Stout

Tested-by: Marek Vasut 

-- 
Best regards,
Marek Vasut

[PATCH] ARM: shmobile: Fix R-Car Gen2 regulator quirk

[Marek Vasut]

The quirk code currently detects all compatible I2C chips with a shared
IRQ line on all I2C busses, adds them into a list, and registers a bus
notifier. For every chip for which the bus notifier triggers, the quirk
code performs I2C transfer on that I2C bus for all addresses in the list.
The problem is that this may generate transfers to non-existing chips on
systems with multiple I2C busses.

This patch adds a check to verify that the I2C bus to which the chip with
shared IRQ is attached to matches the I2C bus of the chip which triggered
the bus notifier and only starts the I2C transfer if they match.

Signed-off-by: Marek Vasut 
Cc: Geert Uytterhoeven 
Cc: Kuninori Morimoto 
Cc: Simon Horman 
Cc: Wolfram Sang 
Cc: Yoshihiro Shimoda 
Cc: linux-renesas-soc@vger.kernel.org
---
 arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c 
b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
index d4774d8ff997..f78e5348bd4c 100644
--- a/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
@@ -40,6 +40,7 @@
 struct regulator_quirk {
struct list_headlist;
const struct of_device_id   *id;
+   struct device_node  *np;
struct of_phandle_args  irq_args;
struct i2c_msg  i2c_msg;
boolshared; /* IRQ line is shared */
@@ -102,6 +103,9 @@ static int regulator_quirk_notify(struct notifier_block *nb,
if (!pos->shared)
continue;
 
+   if (pos->np->parent != client->dev.parent->of_node)
+   continue;
+
dev_info(&client->dev, "clearing %s@0x%02x interrupts\n",
 pos->id->compatible, pos->i2c_msg.addr);
 
@@ -167,6 +171,7 @@ static int __init rcar_gen2_regulator_quirk(void)
memcpy(&quirk->i2c_msg, id->data, sizeof(quirk->i2c_msg));
 
quirk->id = id;
+   quirk->np = np;
quirk->i2c_msg.addr = addr;
 
ret = of_irq_parse_one(np, 0, argsa);
-- 
2.18.0

Re: [git pull] clk: renesas: Updates for v4.21 (take two)

[Stephen Boyd]

Quoting Geert Uytterhoeven (2018-12-07 04:22:03)
> Hi Mike, Stephen,
> 
> The following changes since commit eb38c119dd91c61de26f67050671a84064554f7d:
> 
>   clk: renesas: r7s9210: Add USB clocks (2018-11-13 09:58:51 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git 
> tags/clk-renesas-for-v4.21-tag2
> 
> for you to fetch changes up to 36c4da4f552a126bb29a95dc5c9608795491e32a:
> 
>   clk: renesas: rcar-gen3: Add HS400 quirk for SD clock (2018-12-07 11:45:06 
> +0100)
> 

Thanks. Pulled into clk-next.

Re: [PATCH] arm64: dts: renesas: enable HS400 on R-Car Gen3

[Niklas Söderlund]

Hi Simon, Wolfram,

On 2018-12-05 22:36:13 +0100, Niklas Söderlund wrote:
> Hi Wolfram,
> 
> On 2018-12-05 21:46:28 +0100, Wolfram Sang wrote:
> > On Fri, Nov 02, 2018 at 12:57:19PM +0100, Simon Horman wrote:
> > > On Thu, Nov 01, 2018 at 08:45:29PM +0100, Wolfram Sang wrote:
> > > > 
> > > > > This patch have quiet a few dependencies I'm afraid, let me know if 
> > > > > you 
> > > > > wish to be notified once they all upstream. I don't think it's a good 
> > > > > idea to pick this up before all dependencies are resolved.
> > > > 
> > > > Yes, we should do that. Ping Simon once all dependencies are in next. It
> > > > is still fine to have this patch here in case people want to test. BTW
> > > > did you mention your branch somewhere where you collected all these
> > > > patches to make HS400 working/enabled?
> > > > 
> > > > For this patch already:
> > > > 
> > > > Reviewed-by: Wolfram Sang 
> > > > Tested-by: Wolfram Sang 
> > > 
> > > Thanks, I am marking this as deferred.
> > > 
> > > Please repost or ping me once the dependencies are present in
> > > an rc release.
> > 
> > Niklas, are we done now, so we can ask Simon to pick up the DTS change?
> > 
> 
> Almost :-)
> 
> I'm  waiting for Geert to take the SD quirk patches before pinging Simon 
> to take this one. Spoke to him today about that so I hope it will not be 
> to long.
> 

I just confirmation that the clock patches have been pulled into 
clk-next so this patch is ready to be consumed by you Simon.

Would it be possible to get this in this cycle or is it to late? I know 
you wished to close your tree for v4.21 at the end of this week which it 
technically is now ;-)

-- 
Regards,
Niklas Söderlund

Re: [RFC] watchdog: renesas_wdt: don't keep timer value during suspend/resume

[Wolfram Sang]

Hi Guenter, all,

> > After discussing this mail thread [1] again, we concluded that giving
> > userspace enough time to prepare is our favourite option. So, do not
> > keep the time value when suspended but reset it when resuming.
> > 
> > [1] https://patchwork.kernel.org/patch/10252209/
> > 
> > Signed-off-by: Wolfram Sang 
> 
> Above exchange says it all, no need to repeat.
> 
> Reviewed-by: Guenter Roeck 

Thanks.

I can relate to the policy argument, though. Regardless of this patch, I
wonder if we can make it configurable from userspace. A draft:

#define WDIOF_RESUME_OPTS   0x0800

#define WDIOS_RESUME_KEEP   0x0008
#define WDIOS_RESUME_RESET  0x0010

and then in watchdog_ioctl() under WDIOC_SETOPTIONS:

if (!(wdd->info->options & WDIOF_RESUME_OPTS))
err = -EOPNOTSUPP;
goto break;

if (val & WDIOS_RESUME_KEEP)
wdd->status |= WDOG_KEEP_TIMER_WHEN_RESUME;

if (val & WDIOS_RESUME_RESET)
wdd->status ~= ~WDOG_KEEP_TIMER_WHEN_RESUME;

So, drivers with WDIOF_RESUME_OPTS could act on the
WDOG_KEEP_TIMER_WHEN_RESUME flag.

Opinions?

Thanks,

   Wolfram

pwm: rcar: improve calculation of divider

[Uwe Kleine-König]

Hello,

while looking at the driver I noticed another patch opportunity: In
rcar_pwm_get_clock_division() there is a loop:

for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
(1 << div);
do_div(max, clk_rate);
if (period_ns <= max)
break;
}

The value of div should be calculatable without a loop. Something like:

   divider = NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
   tmp = (unsigned long long)period_ns * clk_rate + (divider - 1);
   do_div(tmp, divider);
   div = ilog2(tmp - 1) + 1;

You might want to check if my maths are right, I didn't test.

Best regards
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/  |

Re: [PATCH 2/3] arm64: defconfig: Enable scu-simple-card driver

[Simon Horman]

On Fri, Dec 07, 2018 at 09:28:22AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Dec 6, 2018 at 10:58 PM Simon Horman  
> wrote:
> > Enable the scu-simple-card which is used by
> > the R-Car E3 (r8a77990) based Ebisu board.
> >
> > Signed-off-by: Simon Horman 
> > ---
> >  arch/arm64/configs/defconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> > index f88190463481..9d0b42d96f03 100644
> > --- a/arch/arm64/configs/defconfig
> > +++ b/arch/arm64/configs/defconfig
> > @@ -491,6 +491,7 @@ CONFIG_SND_SOC_RT5514=m
> >  CONFIG_SND_SOC_RT5514_SPI=m
> >  CONFIG_SND_SOC_RT5645=m
> >  CONFIG_SND_SIMPLE_CARD=m
> > +CONFIG_SND_SIMPLE_SCU_CARD=y
> 
> Shouldn't this be "m"?

Thanks Geert,

I'll look into an incremental update for that.

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

Re: [PATCH v4 2/5] dt-bindings: rtc: pcf85363: Document pcf85263 real-time clock

[Rob Herring]

On Fri,  7 Dec 2018 11:27:43 +, Biju Das wrote:
> The pcf85263 RTC is compatible with the pcf85363 RTC.
> 
> The difference between the pcf85263 and pcf85363 RTC is that the latter has
> 64 bytes more RAM. This renders them incompatible from a DT point of view.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Geert Uytterhoeven 
> ---
> V1-->V2
>   * Incorporated Simon's review comment.
> V2-->V3
>   * No Change.
> V3-->V4
>   * No Change.
> ---
>  Documentation/devicetree/bindings/rtc/pcf85363.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Rob Herring 

Re: [PATCH mmc-utils] fix GCC7 build by refactoring trimming routines

[Chris Ball]

Hi everyone, I'm very sorry for the delay in responding to these patches
and the duplicated effort it caused.

On Mon, Dec 03, 2018 at 03:24:42PM +0100, Wolfram Sang wrote:
> Hi Avri, hi Chris,
> 
> > > I got a compile error with GCC7. When trimming white spaces from strings
> > > lsmmc uses strncpy with overlapping memory areas. This is not allowed.
> > > In addition, the implementation was not efficient with calling strlen
> > > and strncpy once per iteration. Refactor the code to be valid and more
> > > effective.
> > > 
> > > Signed-off-by: Wolfram Sang 

Thanks, I've applied this fix of Wolfram's to mmc-utils master.

> Thanks for the pointer. Chris, how do you feel with maintaining
> mmc-utils? Would you maybe be willing to share maintenance with someone?

Yep, that'd make sense.

-- 
Chris Ball   

Re: [PATCH mmc-utils] use proper type for RPMB blocks_cnt

[Chris Ball]

Hi Wolfram,

On Fri, Nov 30, 2018 at 01:54:47PM +0100, Wolfram Sang wrote:
> The JEDEC standard is confusing. The number of max blocks for reading
> RPMB is determined by CMD23 which can hold an unsigned int and not only
> u16. It is true that the current maximum is 64K of blocks, yet this may
> be extended in the future. Let's not apply a limit here which should be
> checked by the card.
> 
> Signed-off-by: Wolfram Sang 

Thanks, applied to mmc-utils master.

-- 
Chris Ball   

Re: [PATCH 1/8] clk: renesas: Remove usage of CLK_IS_BASIC

[Geert Uytterhoeven]

Hi Stephen,

On Thu, Dec 6, 2018 at 10:59 PM Stephen Boyd  wrote:
> This flag doesn't look to be used by any code, just set in various clk
> init structures and then never tested again. Remove it from these
> drivers as it doesn't provide any benefit.
>
> Cc: Geert Uytterhoeven 
> Cc: 
> Signed-off-by: Stephen Boyd 

Thanks!

Reviewed-by: Geert Uytterhoeven 

I assume you want to apply this yourself, with the rest of the series?

Gr{oetje,eeting}s,

Geert

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

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

[PATCH v3 1/2] spi: Add Renesas R-Car Gen3 RPC SPI controller driver

[Mason Yang]

Add a driver for Renesas R-Car Gen3 RPC SPI controller.

Signed-off-by: Mason Yang 
---
 drivers/spi/Kconfig   |   6 +
 drivers/spi/Makefile  |   1 +
 drivers/spi/spi-renesas-rpc.c | 776 ++
 3 files changed, 783 insertions(+)
 create mode 100644 drivers/spi/spi-renesas-rpc.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 7d3a5c9..54b40f8 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -528,6 +528,12 @@ config SPI_RSPI
help
  SPI driver for Renesas RSPI and QSPI blocks.
 
+config SPI_RENESAS_RPC
+   tristate "Renesas R-Car Gen3 RPC SPI controller"
+   depends on ARCH_RENESAS || COMPILE_TEST
+   help
+ SPI driver for Renesas R-Car Gen3 RPC.
+
 config SPI_QCOM_QSPI
tristate "QTI QSPI controller"
depends on ARCH_QCOM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 3575205..5d5c523 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SPI_QUP) += spi-qup.o
 obj-$(CONFIG_SPI_ROCKCHIP) += spi-rockchip.o
 obj-$(CONFIG_SPI_RB4XX)+= spi-rb4xx.o
 obj-$(CONFIG_SPI_RSPI) += spi-rspi.o
+obj-$(CONFIG_SPI_RENESAS_RPC)  += spi-renesas-rpc.o
 obj-$(CONFIG_SPI_S3C24XX)  += spi-s3c24xx-hw.o
 spi-s3c24xx-hw-y   := spi-s3c24xx.o
 spi-s3c24xx-hw-$(CONFIG_SPI_S3C24XX_FIQ) += spi-s3c24xx-fiq.o
diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
new file mode 100644
index 000..cec5669
--- /dev/null
+++ b/drivers/spi/spi-renesas-rpc.c
@@ -0,0 +1,776 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
+// Copyright (C) 2018 Macronix International Co., Ltd.
+//
+// R-Car Gen3 RPC SPI/QSPI/Octa driver
+//
+// Authors:
+// Mason Yang 
+//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define RPC_CMNCR  0x  /* R/W */
+#define RPC_CMNCR_MD   BIT(31)
+#define RPC_CMNCR_SFDE BIT(24)
+#define RPC_CMNCR_MOIIO3(val)  (((val) & 0x3) << 22)
+#define RPC_CMNCR_MOIIO2(val)  (((val) & 0x3) << 20)
+#define RPC_CMNCR_MOIIO1(val)  (((val) & 0x3) << 18)
+#define RPC_CMNCR_MOIIO0(val)  (((val) & 0x3) << 16)
+#define RPC_CMNCR_MOIIO_HIZ(RPC_CMNCR_MOIIO0(3) | RPC_CMNCR_MOIIO1(3) | \
+RPC_CMNCR_MOIIO2(3) | RPC_CMNCR_MOIIO3(3))
+#define RPC_CMNCR_IO3FV(val)   (((val) & 0x3) << 14)
+#define RPC_CMNCR_IO2FV(val)   (((val) & 0x3) << 12)
+#define RPC_CMNCR_IO0FV(val)   (((val) & 0x3) << 8)
+#define RPC_CMNCR_IOFV_HIZ (RPC_CMNCR_IO0FV(3) | RPC_CMNCR_IO2FV(3) | \
+RPC_CMNCR_IO3FV(3))
+#define RPC_CMNCR_CPHATBIT(6)
+#define RPC_CMNCR_CPHARBIT(5)
+#define RPC_CMNCR_SSLP BIT(4)
+#define RPC_CMNCR_CPOL BIT(3)
+#define RPC_CMNCR_BSZ(val) (((val) & 0x3) << 0)
+
+#define RPC_SSLDR  0x0004  /* R/W */
+#define RPC_SSLDR_SPNDL(d) (((d) & 0x7) << 16)
+#define RPC_SSLDR_SLNDL(d) (((d) & 0x7) << 8)
+#define RPC_SSLDR_SCKDL(d) (((d) & 0x7) << 0)
+
+#define RPC_DRCR   0x000C  /* R/W */
+#define RPC_DRCR_SSLN  BIT(24)
+#define RPC_DRCR_RBURST(v) (((v) & 0x1F) << 16)
+#define RPC_DRCR_RCF   BIT(9)
+#define RPC_DRCR_RBE   BIT(8)
+#define RPC_DRCR_SSLE  BIT(0)
+
+#define RPC_DRCMR  0x0010  /* R/W */
+#define RPC_DRCMR_CMD(c)   (((c) & 0xFF) << 16)
+#define RPC_DRCMR_OCMD(c)  (((c) & 0xFF) << 0)
+
+#define RPC_DREAR  0x0014  /* R/W */
+#define RPC_DREAR_EAC  BIT(0)
+
+#define RPC_DROPR  0x0018  /* R/W */
+
+#define RPC_DRENR  0x001C  /* R/W */
+#define RPC_DRENR_CDB(o)   (u32)o) & 0x3) << 30))
+#define RPC_DRENR_OCDB(o)  (((o) & 0x3) << 28)
+#define RPC_DRENR_ADB(o)   (((o) & 0x3) << 24)
+#define RPC_DRENR_OPDB(o)  (((o) & 0x3) << 20)
+#define RPC_DRENR_SPIDB(o) (((o) & 0x3) << 16)
+#define RPC_DRENR_DME  BIT(15)
+#define RPC_DRENR_CDE  BIT(14)
+#define RPC_DRENR_OCDE BIT(12)
+#define RPC_DRENR_ADE(v)   (((v) & 0xF) << 8)
+#define RPC_DRENR_OPDE(v)  (((v) & 0xF) << 4)
+
+#define RPC_SMCR   0x0020  /* R/W */
+#define RPC_SMCR_SSLKP BIT(8)
+#define RPC_SMCR_SPIRE BIT(2)
+#define RPC_SMCR_SPIWE BIT(1)
+#define RPC_SMCR_SPIE  BIT(0)
+
+#define RPC_SMCMR  0x0024  /* R/W */
+#define RPC_SMCMR_CMD(c)   (((c) & 0xFF) << 16)
+#define RPC_SMCMR_OCMD(c)  (((c) & 0xFF) << 0)
+
+#define RPC_SMADR  0x0028  /* R/W */
+#define RPC_SMOPR  0x002C  /* R/W */
+#define RPC_SMOPR_OPD0(o)  (((o) & 0xFF) << 0)
+#define RPC_SMOPR_OPD1(o)  (((o) & 0xFF) << 8)
+#define RPC_SMOPR_OPD2(o)  (((o) & 0xFF

[PATCH v3 0/2] spi: Add Renesas R-Car Gen3 RPC SPI driver

[Mason Yang]

Hi Mark,

This Renesas R-Car Gen3 RPC SPI driver is based on Boris's new
spi-mem direct mapping read/write mode [1][2].

v3 patch is according to Marek and Geert's comments including:
1) soc_device_mach() to set up RPC_PHYCNT_STRTIM.
2) get_unaligned()
3) rpc-mode for rpi-spi-flash or rpc-hyperflash.
4) coding style and so on.


v2 patch including:
1) remove RPC clock enable/dis-able control,
2) patch run time PM, 
3) add RPC module software reset, 
4) add regmap,
5) other coding style and so on.

thanks for your review.

best regards,
Mason

[1] https://patchwork.kernel.org/patch/10670753
[2] https://patchwork.kernel.org/patch/10670747


Mason Yang (2):
  spi: Add Renesas R-Car Gen3 RPC SPI controller driver
  dt-binding: spi: Document Renesas R-Car Gen3 RPC controller bindings

 .../devicetree/bindings/spi/spi-renesas-rpc.txt|  38 +
 drivers/spi/Kconfig|   6 +
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-renesas-rpc.c  | 776 +
 4 files changed, 821 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
 create mode 100644 drivers/spi/spi-renesas-rpc.c

-- 
1.9.1

[PATCH v3 2/2] dt-binding: spi: Document Renesas R-Car Gen3 RPC controller bindings

[Mason Yang]

Document the bindings used by the Renesas R-Car Gen3 RPC controller.

Signed-off-by: Mason Yang 
---
 .../devicetree/bindings/spi/spi-renesas-rpc.txt| 38 ++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
new file mode 100644
index 000..a191f70
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
@@ -0,0 +1,38 @@
+Renesas R-Car Gen3 RPC controller Device Tree Bindings
+--
+
+Required properties:
+- compatible: should be "renesas,r8a77995-rpc"
+- #address-cells: should be 1
+- #size-cells: should be 0
+- reg: should contain 2 entries, one for the registers and one for the direct
+   mapping area
+- reg-names: should contain "rpc_regs" and "dirmap"
+- interrupts: interrupt line connected to the RPC controller
+- clock-names: should contain "clk_rpc"
+- clocks: should contain 1 entries for the module's clock
+- rpc-mode: should contain "rpc-spi-flash" for rpc spi mode or
+  "rpc-hyperflash" for rpc hyerflash mode.
+
+Example:
+
+   rpc: rpc@ee20 {
+   compatible = "renesas,r8a77995-rpc";
+   reg = <0 0xee20 0 0x8100>, <0 0x0800 0 0x400>;
+   reg-names = "rpc_regs", "dirmap";
+   clocks = <&cpg CPG_MOD 917>;
+   power-domains = <&sysc R8A77995_PD_ALWAYS_ON>;
+   resets = <&cpg 917>;
+   clock-names = "clk_rpc";
+   rpc-mode = "rpc-spi-flash";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   flash@0 {
+   compatible = "jedec,spi-nor";
+   reg = <0>;
+   spi-max-frequency = <4000>;
+   spi-tx-bus-width = <1>;
+   spi-rx-bus-width = <1>;
+   };
+   };
-- 
1.9.1

Re: [PATCH 1/5] drm: rcar-du: Replace EXT_CTRL_REGS feature flag with generation check

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> The RCAR_DU_FEATURE_EXT_CTRL_REGS feature flag is missing for H1 only,
> which is a first generation device, not a second generation device as
> reported in the device information table. Fix the H1 generation and use
> generation checks to replace the feature flag.

Looks like a good simplification

> Signed-off-by: Laurent Pinchart 

Reviewed-by: Kieran Bingham 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   | 14 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |  7 +++
>  drivers/gpu/drm/rcar-du/rcar_du_group.c |  4 ++--
>  3 files changed, 6 insertions(+), 19 deletions(-)

-13 lines ... sounds like a (small) win :D


> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 94f055186b95..814c1099dacb 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -35,7 +35,6 @@
>  static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -58,7 +57,6 @@ static const struct rcar_du_device_info 
> rzg1_du_r8a7743_info = {
>  static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -80,7 +78,6 @@ static const struct rcar_du_device_info 
> rzg1_du_r8a7745_info = {
>  static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -105,7 +102,7 @@ static const struct rcar_du_device_info 
> rzg1_du_r8a77470_info = {
>  };
>  
>  static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> - .gen = 2,
> + .gen = 1,
>   .features = RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -128,7 +125,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7779_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7790_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .quirks = RCAR_DU_QUIRK_ALIGN_128B,
> @@ -158,7 +154,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7790_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7791_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -182,7 +177,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7791_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7792_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -202,7 +196,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7792_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7794_info = {
>   .gen = 2,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
>   .channels_mask = BIT(1) | BIT(0),
> @@ -225,7 +218,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7794_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7795_info = {
>   .gen = 3,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -259,7 +251,6 @@ static const struct rcar_du_device_info 
> rcar_du_r8a7795_info = {
>  static const struct rcar_du_device_info rcar_du_r8a7796_info = {
>   .gen = 3,
>   .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> -   | RCAR_DU_FEATURE_EXT_CTRL_REGS
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> | RCAR_DU_FEATURE_TVM_SYNC,
> @@ -289,7 +280,6 @@ static const struct rcar_du_

Re: [PATCH v2 1/2] spi: Add Renesas R-Car Gen3 RPC SPI controller driver

[Marek Vasut]

On 12/07/2018 08:24 AM, masonccy...@mxic.com.tw wrote:
> 
> Hi Marek,

Hi,

>> >> >> > +
>> >> >> > +         regmap_write(rpc->regmap, RPC_SMWDR0,
>> >> >> > +                 *(u32 *)(tx_buf + pos));
>> >> >>
>> >> >> *(u32 *) cast is probably not needed , fix casts globally.
>> >> >
>> >> > It must have it!
>> >>
>> >> Why ?
>> >
>> > Get a compiler warning due to tx_bug is void *, as Geert replied.
>>
>> The compiler warning is usually an indication that this is something to
>> check, not silence with a type cast.
>>
>> > Using get_unaligned(), patched code would be
>> > -
>> > regmap_write(rpc->regmap, RPC_SMWDR0,
>> >                  get_unaligned((u32 *)(tx_buf + pos)));                
>> > 
>>
>> Do you need the cast if you use get_unaligned() ?
> 
> yes!

Why ? (you can drop one iteration here by explaining why right away,
since I'll ask for that explanation for sure)

>> >> >> > +
>> >> >> > +static const struct regmap_range rpc_spi_volatile_ranges[] = {
>> >> >> > +   regmap_reg_range(RPC_SMRDR0, RPC_SMRDR0),
>> >> >> > +   regmap_reg_range(RPC_SMWDR0, RPC_SMWDR0),
>> >> >>
>> >> >> Why is SMWDR volatile ?
>> >> >
>> >> > No matter is write-back or write-through.
>> >>
>> >> Oh, do you want to assure the SMWDR value is always written directly to
>> >> the hardware ?
>> >
>> > yes,
>> >
>> >>
>> >> btw. I think SMRDR should be precious ?
>> >
>> > so, you think I should drop
>> > regmap_reg_range(RPC_SMWDR0, RPC_SMWDR0)
>>
>> No, I am asking whether SMRDR should be marked precious or not.
>>
> 
> I don't think so,
> precious_reg: the register should not be read outside of
> call from driver, i.e,. a clear on read interrupt status register.

If you read the reg outside of the call from driver, it will mess up the
internal FIFO and the whole driver will get into undefined state, right?
Maybe Mark can chime in ?

>> [...]
>>
>> > CONFIDENTIALITY NOTE:
>> >
>> > This e-mail and any attachments may contain confidential information
>> > and/or personal data, which is protected by applicable laws. Please be
>> > reminded that duplication, disclosure, distribution, or use of this
>> > e-mail (and/or its attachments) or any part thereof is prohibited. If
>> > you receive this e-mail in error, please notify us immediately and
>> > delete this mail as well as its attachment(s) from your system. In
>> > addition, please be informed that collection, processing, and/or use of
>> > personal data is prohibited unless expressly permitted by personal data
>> > protection laws. Thank you for your attention and cooperation.
>> >
>> > Macronix International Co., Ltd.
>>
>> Can you do something about this ^ please ?
>>
> 
> I submit the patch file is by another remote git-server, which
> supports git-format patch, git send-mail and so on.
> But it can't receive email.
> 
> I get/send email are by office PC w/ Notes system and
> this "CONFx NOTE:" is appended automatically by Notes mail-server.
> 
> Please just never mind it.

I am concerned MXIC can sue everyone on the CC list of this email based
solely on your suggestion to ignore this paragraph. That's not a good
position to be in.

-- 
Best regards,
Marek Vasut

Re: [PATCH 2/5] drm: rcar-du: Move CRTC outputs bitmask to private CRTC state

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> The rcar_du_crtc outputs field stores a bitmask of the outputs driven by
> the CRTC. This changes based on the configuration requested by
> userspace, and is used for the sole purpose of configuring the hardware.
> The field thus belongs to the CRTC state. Move it to the
> rcar_du_crtc_state structure.
> 
> As a result the rcar_du_crtc_route_output() function loses most of its
> purpose. In order to remove it, move dpad0_source calculation to
> rcar_du_atomic_commit_tail(), until the field gets moved to a state
> structure. In order to simplify the rcar_du_group_set_routing()
> implementation, we also store the DPAD1 source in a new dpad1_source
> field which will move to a state structure with dpad0_source.
> 
> Signed-off-by: Laurent Pinchart 

that was a fairly tough read - but aside from one comment near the
bottom regarding initialising dpad0 which I'm sure you can handle
correctly, and another comment which I think we could improve things
outside of this patch:

Reviewed-by: Kieran Bingham 


> ---
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c| 42 +++
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.h|  7 ++--
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h |  1 +
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 10 --
>  drivers/gpu/drm/rcar-du/rcar_du_group.c   |  4 +--
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 22 
>  6 files changed, 47 insertions(+), 39 deletions(-)

+8 ... It's a good job you bought -13 lines in the previous patch ;)



> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 90dacab67be5..40b7f17159b0 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -22,6 +22,7 @@
>  
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
> +#include "rcar_du_encoder.h"
>  #include "rcar_du_kms.h"
>  #include "rcar_du_plane.h"
>  #include "rcar_du_regs.h"
> @@ -316,26 +317,6 @@ static void rcar_du_crtc_set_display_timing(struct 
> rcar_du_crtc *rcrtc)
>   rcar_du_crtc_write(rcrtc, DEWR,  mode->hdisplay);
>  }
>  
> -void rcar_du_crtc_route_output(struct drm_crtc *crtc,
> -enum rcar_du_output output)
> -{
> - struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> - struct rcar_du_device *rcdu = rcrtc->group->dev;
> -
> - /*
> -  * Store the route from the CRTC output to the DU output. The DU will be
> -  * configured when starting the CRTC.
> -  */
> - rcrtc->outputs |= BIT(output);
> -
> - /*
> -  * Store RGB routing to DPAD0, the hardware will be configured when
> -  * starting the CRTC.
> -  */
> - if (output == RCAR_DU_OUTPUT_DPAD0)
> - rcdu->dpad0_source = rcrtc->index;
> -}
> -
>  static unsigned int plane_zpos(struct rcar_du_plane *plane)
>  {
>   return plane->plane.state->normalized_zpos;
> @@ -655,6 +636,24 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc)
>   * CRTC Functions
>   */
>  
> +static int rcar_du_crtc_atomic_check(struct drm_crtc *crtc,
> +  struct drm_crtc_state *state)
> +{
> + struct rcar_du_crtc_state *rstate = to_rcar_crtc_state(state);
> + struct drm_encoder *encoder;
> +
> + /* Store the routes from the CRTC output to the DU outputs. */
> + rstate->outputs = 0;
> +
> + drm_for_each_encoder_mask(encoder, crtc->dev, state->encoder_mask) {
> + struct rcar_du_encoder *renc = to_rcar_encoder(encoder);
> +
> + rstate->outputs |= BIT(renc->output);
> + }
> +
> + return 0;
> +}
> +
>  static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
>  struct drm_crtc_state *old_state)
>  {
> @@ -678,8 +677,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc 
> *crtc,
>   crtc->state->event = NULL;
>   }
>   spin_unlock_irq(&crtc->dev->event_lock);
> -
> - rcrtc->outputs = 0;
>  }
>  
>  static void rcar_du_crtc_atomic_begin(struct drm_crtc *crtc,
> @@ -755,6 +752,7 @@ enum drm_mode_status rcar_du_crtc_mode_valid(struct 
> drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs crtc_helper_funcs = {
> + .atomic_check = rcar_du_crtc_atomic_check,
>   .atomic_begin = rcar_du_crtc_atomic_begin,
>   .atomic_flush = rcar_du_crtc_atomic_flush,
>   .atomic_enable = rcar_du_crtc_atomic_enable,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h 
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> index 59ac6e7d22c9..ec47f164e69b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -37,7 +37,6 @@ struct rcar_du_vsp;
>   * @vblank_lock: protects vblank_wait and vblank_count
>   * @vblank_wait: wait queue used to signal vertical blanking
>   * @vblank_count: number of vertical blanking interrupts to wait for
> - * @outputs: bitmask of the ou

Re: [PATCH 3/5] drm: rcar-du: Disable unused DPAD outputs

[Kieran Bingham]

Hi Laurent,

Thank you for the patch,

On 25/11/2018 14:40, Laurent Pinchart wrote:
> DU channels are routed to DPAD outputs in an SoC-dependent way. The
> routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
> or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
> DPAD outputs, which are thus always driven by a DU channel.
> 
> On SoCs that have less DU channels than DU outputs, such as D3 and E3,
> the DPAD output is always driven when all channels are in used by other

s/used/use/


> outputs (such as the internal LVDS and HDMI encoders). This creates an
> unwanted clone on the DPAD output.
> 
> However, the parallel output of the DU channels routed to DPAD can be
> set to fixed levels in the DU channels themselves through the DOFLR
> group register. Use this to turn the DPAD on or off by driving fixed
> signals at the output of any DU channel not routed to a DPAD output.
> This doesn't affect the DU output signals going to other outputs.
> 
> Signed-off-by: Laurent Pinchart 

Only spelling and bikeshedding here - so:

Reviewed-by: Kieran Bingham 

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_group.c | 42 +
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 7e440f61977f..5aaf41b7a2ca 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device 
> *rcdu)
>   return 0;
>  }
>  
> +static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
> +{
> + static const u32 doflr_values[2] = {
> + DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
> + DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
> + DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
> + DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
> + };
> + static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
> +| BIT(RCAR_DU_OUTPUT_DPAD0);
> + struct rcar_du_device *rcdu = rgrp->dev;
> + u32 doflr = DOFLR_CODE;
> + unsigned int i;
> +
> + if (rcdu->info->gen < 2)
> + return;
> +
> + /*
> +  * The DPAD outputs can't be controlled directly. However, the parallel
> +  * output of the DU channels routed to DPAD can be set to fixed levels
> +  * through the DOFLR group register. Use this to turn the DPAD on or off
> +  * by driving fixed signals at the output of any DU channel not routed
> +  * to a DPAD output. This doesn't affect the DU output signals going to
> +  * other outputs, such as the internal LVDS and HDMI encoders.

Perhaps more out of interest - what /fixed/ levels do we output.
High/Low/Hi-Z ?


> +  */
> +
> + for (i = 0; i < rgrp->num_crtcs; ++i) {
> + struct rcar_du_crtc_state *rstate;
> + struct rcar_du_crtc *rcrtc;
> +
> + rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];> +   rstate 
> = to_rcar_crtc_state(rcrtc->crtc.state);
> +
> + if (!(rstate->outputs & dpad_mask))
> + doflr |= doflr_values[i];> +}
> +
> + rcar_du_group_write(rgrp, DOFLR, doflr);
> +}
> +
>  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  {
>   struct rcar_du_device *rcdu = rgrp->dev;
> @@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
>  
>   rcar_du_group_write(rgrp, DORCR, dorcr);
>  
> + rcar_du_group_set_output_levels(rgrp);

Shouldn't this be:

  rcar_du_group_set_dpad_levels()

Anyway - that's just bikeshedding - I'll leave the decision (even if
that's keeping this as is) to you.

> +
>   return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
>  }
> 

-- 
Regards
--
Kieran

Re: [PATCH v3 2/2] dt-binding: spi: Document Renesas R-Car Gen3 RPC controller bindings

[Sergei Shtylyov]

Hello!

On 12/07/2018 02:13 PM, Mason Yang wrote:

> Document the bindings used by the Renesas R-Car Gen3 RPC controller.
> 
> Signed-off-by: Mason Yang 
> ---
>  .../devicetree/bindings/spi/spi-renesas-rpc.txt| 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
> b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> new file mode 100644
> index 000..a191f70
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> @@ -0,0 +1,38 @@
> +Renesas R-Car Gen3 RPC controller Device Tree Bindings
> +--
> +
> +Required properties:
> +- compatible: should be "renesas,r8a77995-rpc"
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +- reg: should contain 2 entries, one for the registers and one for the direct
> +   mapping area
> +- reg-names: should contain "rpc_regs" and "dirmap"

   Please drop this "rpc_" thing, I think "regs" should be enough.

> +- interrupts: interrupt line connected to the RPC controller
> +- clock-names: should contain "clk_rpc"

   Please drop this "clk_" thing. BTW, what's with the RPCD2 clock?

> +- clocks: should contain 1 entries for the module's clock
> +- rpc-mode: should contain "rpc-spi-flash" for rpc spi mode or
> +"rpc-hyperflash" for rpc hyerflash mode.

   I think the prop should be called "renesas,rpc-mode" and the values should be
just "spi" and "hyperflash". 

[...]

MBR, Sergei

Re: [PATCH v3 2/2] dt-binding: spi: Document Renesas R-Car Gen3 RPC controller bindings

[Marek Vasut]

On 12/07/2018 05:03 PM, Sergei Shtylyov wrote:
> Hello!
> 
> On 12/07/2018 02:13 PM, Mason Yang wrote:
> 
>> Document the bindings used by the Renesas R-Car Gen3 RPC controller.
>>
>> Signed-off-by: Mason Yang 
>> ---
>>  .../devicetree/bindings/spi/spi-renesas-rpc.txt| 38 
>> ++
>>  1 file changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt 
>> b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> new file mode 100644
>> index 000..a191f70
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> @@ -0,0 +1,38 @@
>> +Renesas R-Car Gen3 RPC controller Device Tree Bindings
>> +--
>> +
>> +Required properties:
>> +- compatible: should be "renesas,r8a77995-rpc"
>> +- #address-cells: should be 1
>> +- #size-cells: should be 0
>> +- reg: should contain 2 entries, one for the registers and one for the 
>> direct
>> +   mapping area
>> +- reg-names: should contain "rpc_regs" and "dirmap"
> 
>Please drop this "rpc_" thing, I think "regs" should be enough.
> 
>> +- interrupts: interrupt line connected to the RPC controller
>> +- clock-names: should contain "clk_rpc"
> 
>Please drop this "clk_" thing. BTW, what's with the RPCD2 clock?
> 
>> +- clocks: should contain 1 entries for the module's clock
>> +- rpc-mode: should contain "rpc-spi-flash" for rpc spi mode or
>> +   "rpc-hyperflash" for rpc hyerflash mode.
> 
>I think the prop should be called "renesas,rpc-mode" and the values should 
> be
> just "spi" and "hyperflash". 

Like I said before, you can determine the mode from the subnode attached
to the controller, we don't need special prop for that.

-- 
Best regards,
Marek Vasut

[PATCH] media: vsp1: Fix trivial documentation

[Kieran Bingham]

In the partition sizing the term 'prevents' is inappropriately
pluralized.  Simplify to 'prevent'.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_video.c 
b/drivers/media/platform/vsp1/vsp1_video.c
index 771dfe1f7c20..7ceaf3222145 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -223,7 +223,7 @@ static void vsp1_video_calculate_partition(struct 
vsp1_pipeline *pipe,
 * If the modulus is less than half of the partition size,
 * the penultimate partition is reduced to half, which is added
 * to the final partition: |1234|1234|1234|12|341|
-* to prevents this:   |1234|1234|1234|1234|1|.
+* to prevent this:|1234|1234|1234|1234|1|.
 */
if (modulus) {
/*
-- 
2.17.1

Re: [PATCH v2 1/2] spi: Add Renesas R-Car Gen3 RPC SPI controller driver

[Sergei Shtylyov]

Hello!

   I'd already started the v2 driver review before you posted v3, so here 
goes...

On 12/03/2018 12:18 PM, Mason Yang wrote:

> Add a driver for Renesas R-Car Gen3 RPC SPI controller.
> 
> Signed-off-by: Mason Yang 
[...]
> diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
> new file mode 100644
> index 000..ac9094e
> --- /dev/null
> +++ b/drivers/spi/spi-renesas-rpc.c
> @@ -0,0 +1,808 @@
[...]
> +#define RPC_CMNCR0x  /* R/W */
> +#define RPC_CMNCR_MD BIT(31)
> +#define RPC_CMNCR_SFDE   BIT(24)

   This bit is undocumented as of the gen3 manual v1.0. I'd like this to be 
reflected
in a comment...

[...]
> +#define RPC_CMNCR_IO3FV(val) (((val) & 0x3) << 14)
> +#define RPC_CMNCR_IO2FV(val) (((val) & 0x3) << 12)

   These 2 are also undocumented.

[...]
> +#define RPC_CMNCR_CPHAT  BIT(6)
> +#define RPC_CMNCR_CPHAR  BIT(5)
> +#define RPC_CMNCR_SSLP   BIT(4)
> +#define RPC_CMNCR_CPOL   BIT(3)

   And these 4...

> +#define RPC_DRCR 0x000C  /* R/W */
> +#define RPC_DRCR_SSLNBIT(24)
> +#define RPC_DRCR_RBURST(v)   (((v) & 0x1F) << 16)

   More like ((v) - 1), like in another register...

> +#define RPC_DRCR_RCF BIT(9)
> +#define RPC_DRCR_RBE BIT(8)
> +#define RPC_DRCR_SSLEBIT(0)
[...]
> +#define RPC_DREAR0x0014  /* R/W */
> +#define RPC_DREAR_EACBIT(0)

   The manual says it takes bits 0 to 2...

[...]
> +#define RPC_SMADR0x0028  /* R/W */
> +#define RPC_SMOPR0x002C  /* R/W */
> +#define RPC_SMOPR_OPD0(o)(((o) & 0xFF) << 0)
> +#define RPC_SMOPR_OPD1(o)(((o) & 0xFF) << 8)
> +#define RPC_SMOPR_OPD2(o)(((o) & 0xFF) << 16)
> +#define RPC_SMOPR_OPD3(o)(((o) & 0xFF) << 24)

  You either go in descending or ascending order, not both. :-)

[...]
> +#define RPC_PHYCNT   0x007C  /* R/W */
> +#define RPC_PHYCNT_CAL   BIT(31)
> +#define PRC_PHYCNT_OCTA_AA   BIT(22)
> +#define PRC_PHYCNT_OCTA_SA   BIT(23)
> +#define PRC_PHYCNT_EXDS  BIT(21)
> +#define RPC_PHYCNT_OCT   BIT(20)
> +#define RPC_PHYCNT_STRTIM(v) (((v) & 0x7) << 15)
> +#define RPC_PHYCNT_WBUF2 BIT(4)
> +#define RPC_PHYCNT_WBUF  BIT(2)
> +#define RPC_PHYCNT_MEM(v)(((v) & 0x3) << 0)

   The manual calls this field PHYMEM...

[...]
> +#define LOOP_TIMEOUT 1024

   It's more like TIMEOUT_LOOPS. :-)

[...]
> +static void rpc_spi_hw_init(struct rpc_spi *rpc)
> +{
> + /*
> +  * NOTE: The 0x260 are undocumented bits, but they must be set.
> +  *  RPC_PHYCNT_STRTIM is strobe timing adjustment bit,
> +  *  0x0 : the delay is biggest,
> +  *  0x1 : the delay is 2nd biggest,
> +  *  0x3 or 0x6 is a recommended value.
> +  */
> + regmap_write(rpc->regmap, RPC_PHYCNT, RPC_PHYCNT_CAL |
> +   RPC_PHYCNT_STRTIM(0x6) | 0x260);
> +
> + /*
> +  * NOTE: The 0x31511144 and 0x431 are undocumented bits,
> +  *   but they must be set for RPC_PHYOFFSET1 & RPC_PHYOFFSET2.
> +  */
> + regmap_write(rpc->regmap, RPC_PHYOFFSET1, 0x31511144);
> + regmap_write(rpc->regmap, RPC_PHYOFFSET2, 0x431);

   0x400 is actually documented, bits 0..7 are read only and default to 0x31...

> +#ifdef CONFIG_RESET_CONTROLLER
> +static int rpc_spi_do_reset(struct rpc_spi *rpc)
> +{
> + int i, ret;
> +
> + ret = reset_control_reset(rpc->rstc);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < LOOP_TIMEOUT; i++) {
> + ret = reset_control_status(rpc->rstc);
> + if (ret == 0)
> + return 0;
> + usleep_range(0, 1);

   Are you serious? :-)

> + }
> +
> + return -ETIMEDOUT;
> +}
> +#else
> +static int rpc_spi_do_reset(struct rpc_spi *rpc)
> +{
> + return -ETIMEDOUT;
> +}
> +#endif
[...]
> +static int rpc_spi_io_xfer(struct rpc_spi *rpc,
> +const void *tx_buf, void *rx_buf)
> +{
> + u32 smenr, smcr, data, pos = 0;
> + int ret = 0;
> +
> + regmap_write(rpc->regmap, RPC_CMNCR, RPC_CMNCR_MD | RPC_CMNCR_SFDE |
> +   RPC_CMNCR_MOIIO_HIZ | RPC_CMNCR_IOFV_HIZ |
> +   RPC_CMNCR_BSZ(0));
> + regmap_write(rpc->regmap, RPC_SMDRENR, 0x0);

   Just 0, please.

> + regmap_write(rpc->regmap, RPC_SMCMR, rpc->cmd);
> + regmap_write(rpc->regmap, RPC_SMDMCR, rpc->dummy);
> + regmap_write(rpc->regmap, RPC_SMADR, rpc->addr);
> +
> + if (tx_buf) {
> + smenr = rpc->smenr;
> +
> + while (pos < rpc->xferlen) {
> + u32 nbytes = rpc->xferlen  - pos;

   One space before '-' is enough. :-)

> +
> + regmap_write(rpc->regmap, RPC_SMWDR0,
> +  *(u32 *)(tx_buf + pos));

   Ugh... what about the data endianness?

> +
> +  

Re: [PATCH v2 1/2] spi: Add Renesas R-Car Gen3 RPC SPI controller driver

[Marek Vasut]

On 12/07/2018 07:17 PM, Sergei Shtylyov wrote:
> Hello!
> 
>I'd already started the v2 driver review before you posted v3, so here 
> goes...
> 
> On 12/03/2018 12:18 PM, Mason Yang wrote:
> 
>> Add a driver for Renesas R-Car Gen3 RPC SPI controller.
>>
>> Signed-off-by: Mason Yang 
> [...]
>> diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
>> new file mode 100644
>> index 000..ac9094e
>> --- /dev/null
>> +++ b/drivers/spi/spi-renesas-rpc.c
>> @@ -0,0 +1,808 @@
> [...]
>> +#define RPC_CMNCR   0x  /* R/W */
>> +#define RPC_CMNCR_MDBIT(31)
>> +#define RPC_CMNCR_SFDE  BIT(24)
> 
>This bit is undocumented as of the gen3 manual v1.0. I'd like this to be 
> reflected
> in a comment...

FYI, not even in v1.50 .

-- 
Best regards,
Marek Vasut

Re: [PATCH 1/8] clk: renesas: Remove usage of CLK_IS_BASIC

[Stephen Boyd]

Quoting Geert Uytterhoeven (2018-12-07 00:27:11)
> Hi Stephen,
> 
> On Thu, Dec 6, 2018 at 10:59 PM Stephen Boyd  wrote:
> > This flag doesn't look to be used by any code, just set in various clk
> > init structures and then never tested again. Remove it from these
> > drivers as it doesn't provide any benefit.
> >
> > Cc: Geert Uytterhoeven 
> > Cc: 
> > Signed-off-by: Stephen Boyd 
> 
> Thanks!
> 
> Reviewed-by: Geert Uytterhoeven 
> 
> I assume you want to apply this yourself, with the rest of the series?
> 

Yes I'll apply it to clk-next. Thanks.

Re: [PATCH 3/5] drm: rcar-du: Disable unused DPAD outputs

[Laurent Pinchart]

Hi Kieran,

On Friday, 7 December 2018 14:50:47 EET Kieran Bingham wrote:
> On 25/11/2018 14:40, Laurent Pinchart wrote:
> > DU channels are routed to DPAD outputs in an SoC-dependent way. The
> > routing can be fixed (e.g. DU3 to DPAD0 on H3) or configurable (e.g. DU0
> > or DU1 to DPAD0 on D3/E3). The hardware offers no option to disconnect
> > DPAD outputs, which are thus always driven by a DU channel.
> > 
> > On SoCs that have less DU channels than DU outputs, such as D3 and E3,
> > the DPAD output is always driven when all channels are in used by other
> 
> s/used/use/
> 
> > outputs (such as the internal LVDS and HDMI encoders). This creates an
> > unwanted clone on the DPAD output.
> > 
> > However, the parallel output of the DU channels routed to DPAD can be
> > set to fixed levels in the DU channels themselves through the DOFLR
> > group register. Use this to turn the DPAD on or off by driving fixed
> > signals at the output of any DU channel not routed to a DPAD output.
> > This doesn't affect the DU output signals going to other outputs.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> 
> Only spelling and bikeshedding here - so:
> 
> Reviewed-by: Kieran Bingham 
> 
> > ---
> > 
> >  drivers/gpu/drm/rcar-du/rcar_du_group.c | 42 +
> >  1 file changed, 42 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_group.c index
> > 7e440f61977f..5aaf41b7a2ca 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> > @@ -287,6 +287,46 @@ int rcar_du_set_dpad0_vsp1_routing(struct
> > rcar_du_device *rcdu)
> > return 0;
> >  }
> > 
> > +static void rcar_du_group_set_output_levels(struct rcar_du_group *rgrp)
> > +{
> > +   static const u32 doflr_values[2] = {
> > +   DOFLR_HSYCFL0 | DOFLR_VSYCFL0 | DOFLR_ODDFL0 |
> > +   DOFLR_DISPFL0 | DOFLR_CDEFL0  | DOFLR_RGBFL0,
> > +   DOFLR_HSYCFL1 | DOFLR_VSYCFL1 | DOFLR_ODDFL1 |
> > +   DOFLR_DISPFL1 | DOFLR_CDEFL1  | DOFLR_RGBFL1,
> > +   };
> > +   static const u32 dpad_mask = BIT(RCAR_DU_OUTPUT_DPAD1)
> > +  | BIT(RCAR_DU_OUTPUT_DPAD0);
> > +   struct rcar_du_device *rcdu = rgrp->dev;
> > +   u32 doflr = DOFLR_CODE;
> > +   unsigned int i;
> > +
> > +   if (rcdu->info->gen < 2)
> > +   return;
> > +
> > +   /*
> > +* The DPAD outputs can't be controlled directly. However, the parallel
> > +* output of the DU channels routed to DPAD can be set to fixed levels
> > +* through the DOFLR group register. Use this to turn the DPAD on or 
off
> > +* by driving fixed signals at the output of any DU channel not routed
> > +* to a DPAD output. This doesn't affect the DU output signals going to
> > +* other outputs, such as the internal LVDS and HDMI encoders.
> 
> Perhaps more out of interest - what /fixed/ levels do we output.
> High/Low/Hi-Z ?

It's low-level, I'll update the comment.

> > +*/
> > +
> > +   for (i = 0; i < rgrp->num_crtcs; ++i) {
> > +   struct rcar_du_crtc_state *rstate;
> > +   struct rcar_du_crtc *rcrtc;
> > +
> > +   rcrtc = &rcdu->crtcs[rgrp->index * 2 + i];> +   rstate =
> > to_rcar_crtc_state(rcrtc->crtc.state); +
> > +   if (!(rstate->outputs & dpad_mask))
> > +   doflr |= doflr_values[i];> +}
> > +
> > +   rcar_du_group_write(rgrp, DOFLR, doflr);
> > +}
> > +
> > 
> >  int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
> >  {
> >  
> > struct rcar_du_device *rcdu = rgrp->dev;
> > 
> > @@ -306,5 +346,7 @@ int rcar_du_group_set_routing(struct rcar_du_group
> > *rgrp)> 
> > rcar_du_group_write(rgrp, DORCR, dorcr);
> > 
> > +   rcar_du_group_set_output_levels(rgrp);
> 
> Shouldn't this be:
> 
>   rcar_du_group_set_dpad_levels()
> 
> Anyway - that's just bikeshedding - I'll leave the decision (even if
> that's keeping this as is) to you.

Good idea, I'll rename the function.

> > +
> > 
> > return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
> >  
> >  }


-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/4] dt-bindings: adv748x: make data-lanes property mandatory for CSI-2 endpoints

[Rob Herring]

On Thu, 29 Nov 2018 03:01:44 +0100, =?UTF-8?q?Niklas=20S=C3=B6derlund?= 
 wrote:
> The CSI-2 transmitters can use a different number of lanes to transmit
> data. Make the data-lanes mandatory for the endpoints that describe the
> transmitters as no good default can be set to fallback on.
> 
> Signed-off-by: Niklas Söderlund 
> 
> ---
> * Changes since v3
> - Add paragraph to describe the accepted values for the source endpoint
>   data-lane property. Thanks Jacopo for pointing this out and sorry for
>   missing this in v2.
> * Changes since v2
> - Update paragraph according to Laurents comments.
> ---
>  .../devicetree/bindings/media/i2c/adv748x.txt | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring