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: [PATCH v2 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Wolfram,

Thanks for your feedback.

On 2018-11-29 17:54:34 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> thanks for the patches!
> 
> On Thu, Nov 29, 2018 at 01:39:49AM +0100, Niklas Söderlund wrote:
> > On H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1) the clock setting for HS400
> > needs a quirk to function properly. The reason for the quirk is that
> > there are two settings which produces same divider value for the SDn
> > clock. On the effected boards the one currently selected results in
> > HS400 not working.
> > 
> > This change uses the same method as the Gen2 CPG driver and simply
> > ignores the first clock setting as this is the offending one when
> > selecting the settings. Which of the two possible settings is used have
> > no effect for SDR104.
> > 
> > Signed-off-by: Niklas Söderlund 
> > 
> > ---
> > * Changes since v1
> > - Fixed spelling in commit message, thanks Sergei and Geert!
> > - Reworked the whole patch per Geerts suggestion. Instead of only
> >   skipping the first row on the effected boards when setting the clock
> >   rete totally ignore it. This is made possible by another change to the
> 
> "rete"? I don't get this sentence and I think it is important to
> understand when reviewing these patches :)

That should have been rate :-) To elaborate a bit more:

The patch is different from v1 as a different approach to solve the 
issue have been found. Instead of only ignoring the first row of the 
list of possible settings when selecting which divider to use also 
ignore it when examining which state the hardware is in. That is the 
driver is no longer aware the first row exists with this patch.

This was in v1 not possible as the first row might be a state the 
bootloader left the hardware in and then the clock failed to register as 
it would need to update its own state to match the hardware.

As the driver needed to know about the state the hardware was in when 
probing but not use it when selecting a divider the more complex v1 was 
needed. When selecting a divider we wish for it to select the second 
option for the divider value '4' when running on a SoC which needs the 
quirk.

With v2 which depends on [1] this is not needed as the clock driver now 
sets a know state when registering the clock so this patch can be made 
much simpler by simply 'removing' the first row from all operations.

> 
> >   clock driver posted separately from this series and which this patch
> >   now depends on [1].
> 
> Hmm, why didn't you add it to the series then?

Since it is unrelated to this series I thought it best to post it as a 
separate patch as I think it has value to create a known starting state 
disregarding where this series ends up :-)

> 
> Still, all in all, seems we are on a nice track for having HS400 in the
> next release \o/ Now, if that doesn't justify the 5.0 jump... ;D

I sure hope so!

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Geert,

On 2018-11-28 19:02:33 +0100, Niklas Söderlund wrote:
> Hi Geert,
> 
> Thanks for your feedback.
> 
> On 2018-11-05 16:45:39 +0100, Geert Uytterhoeven wrote:
> > Hi Niklas,
> > 
> > On Mon, Nov 5, 2018 at 4:07 PM Niklas Söderlund
> >  wrote:
> > > On 2018-11-05 11:43:24 +0100, Geert Uytterhoeven wrote:
> > > > On Thu, Nov 1, 2018 at 12:26 AM Niklas Söderlund
> > > >  wrote:
> > > > > From: Niklas Söderlund 
> > > > >
> > > > > On H3 (ES1.0,ES2.0) and M3-W (ES1.0,ES1.1) the clock setting for HS400
> > > >
> > > > H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1)
> > >
> > > Thanks.
> > >
> > > >
> > > > > needs a quirk to function properly. The reason for the quirk is that
> > > > > there are two settings which produces same divider vale for the SDn
> > > > > clock. On the effected boards the one currently selected results in 
> > > > > HS00
> > > >
> > > > HS200 or HS400?
> > >
> > > Wops, HS400 :-)
> > >
> > > >
> > > > > not working.
> > > > >
> > > > > This change uses the same method as the Gen2 CPG driver and simply
> > > > > ignores the first clock setting as this is the offending one when
> > > > > selecting the settings. Which of the two possible settings is used 
> > > > > have
> > > > > no effect for SDR104.
> > > > >
> > > > > Signed-off-by: Niklas Söderlund 
> > > > > 
> > > > > ---
> > > > >  drivers/clk/renesas/rcar-gen3-cpg.c | 28 ++--
> > > > >  1 file changed, 22 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
> > > > > b/drivers/clk/renesas/rcar-gen3-cpg.c
> > > > > index ff56ac6134111c38..8cc524a29c855dd2 100644
> > > > > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > > > > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> > 
> > > > > @@ -377,6 +382,7 @@ static struct clk * __init 
> > > > > cpg_sd_clk_register(const struct cpg_core_clk *core,
> > > > > clock->hw.init = 
> > > > > clock->div_table = cpg_sd_div_table;
> > > > > clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
> > > > > +   clock->skip_first = skip_first;
> > > >
> > > > What about
> > > >
> > > > if (cpg_quirks & SD_SKIP_FIRST) {
> > > > clock->div_table++;
> > > > clock->div_num--;
> > > > }
> > > >
> > > > instead?
> > >
> > > This was my first approach as well, unfortunately it won't work :-(
> > >
> > > If the bootloader leaves the clock settings in a state which matches the
> > > first row in the table the clock fails to register as the check in
> > > cpg_sd_clk_register() makes sure it have a row for the state the
> > > hardware is in. If it can't find that state the clock fails to register.
> > > Whit this quirk the limitation of the effected boards only have an
> > > effect when setting the clock.
> > 
> > IC...
> > 
> > > I thought this solution solved this quiet neatly with the robustness
> > > principle, 'Be conservative in what you do, be liberal in what you
> > > accept from others' :-)
> > 
> > Will it ever use the settings as inherited from boot loader or reset state?
> > If it does, I assume it will fail badly, due to an inconsistency between the
> > SD and SDH clock rates?
> > And what if the kernel is ever booted when the SDnSRCFC or SDnFC field
> > has an invalid value? Then the driver will fail, too?
> > 
> > Hence, isn't it safer to initialize the registers to a known safe value?
> 
> I agree with you that we should not trust the value fro the bootloader 
> and the sdhi driver do not trust this and resets it. The problem is 
> rcar-gen3-cpg. From cpg_sd_clk_register()
> 
>  sd_fc = readl(clock->csn.reg) & CPG_SD_FC_MASK;
>  for (i = 0; i < clock->div_num; i++)
>  if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
>  break;
> 
>  if (WARN_ON(i >= clock->div_num)) {
>  kfree(clock);
>  return ERR_PTR(-EINVAL);
>      }
> 
>  clock->cur_div_idx = i;
> 
> If the bootloader sets the register to a value not known by the clock 
> driver or if we us the moth you prefer to modify clock->div_table and 
> clock->div_num the clock driver would need to set a safe default. I 
> think this would be fine as the SDHI driver could handle this (I need to 
> test it tho). My proposal is therefor that I add a patch to this series 
> where the clock driver try to set a known divider when it registers the 
> clock.
> 
> My suggestion would be that the divider to be set to 4 as this appears 
> to be what current boot loaders do. Would this work for you?

I have now created a patch which do almost this [1]. But instead of 
setting the divider to 4 just use the first row of clock->div_table and 
make sure the clock is stopped. Any user of the clock needs to set the 
rate it expects before enabling the clock. I tested this on H3 ES1 and 
ES2 and M3-N and it works just fine.

1. [PATCH] clk: renesas: rcar-gen3: set state when registering SD clocks

-- 
Regards,
Niklas Söderlund

[PATCH v2 1/2] clk: renesas: rcar-gen3: add documentation for SD clocks

[Niklas Söderlund]

Document the known use cases of the different clock settings. This is
useful as different SoC and ES versions uses different settings to do
the same thing as there are more then one combination to achieve the
same SDn clock speed.

Signed-off-by: Niklas Söderlund 
Reviewed-by: Wolfram Sang 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index f6335357c85505df..bca6c7f51de18db7 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -232,13 +232,13 @@ struct sd_clock {
  * sd_srcfc   sd_fc   div
  * stp_hck   stp_ck(div)  (div) = sd_srcfc x sd_fc
  *---
- *  0 0 0 (1)  1 (4)  4
- *  0 0 1 (2)  1 (4)  8
- *  1 0 2 (4)  1 (4) 16
- *  1 0 3 (8)  1 (4) 32
+ *  0 0 0 (1)  1 (4)  4 : SDR104 / HS200 / HS400 (8 
TAP)
+ *  0 0 1 (2)  1 (4)  8 : SDR50
+ *  1 0 2 (4)  1 (4) 16 : HS / SDR25
+ *  1 0 3 (8)  1 (4) 32 : NS / SDR12
  *  1 0 4 (16) 1 (4) 64
  *  0 0 0 (1)  0 (2)  2
- *  0 0 1 (2)  0 (2)  4
+ *  0 0 1 (2)  0 (2)  4 : SDR104 / HS200 / HS400 (4 
TAP)
  *  1 0 2 (4)  0 (2)  8
  *  1 0 3 (8)  0 (2) 16
  *  1 0 4 (16) 0 (2) 32
-- 
2.19.1

[PATCH v2 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

On H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1) the clock setting for HS400
needs a quirk to function properly. The reason for the quirk is that
there are two settings which produces same divider value for the SDn
clock. On the effected boards the one currently selected results in
HS400 not working.

This change uses the same method as the Gen2 CPG driver and simply
ignores the first clock setting as this is the offending one when
selecting the settings. Which of the two possible settings is used have
no effect for SDR104.

Signed-off-by: Niklas Söderlund 

---
* Changes since v1
- Fixed spelling in commit message, thanks Sergei and Geert!
- Reworked the whole patch per Geerts suggestion. Instead of only
  skipping the first row on the effected boards when setting the clock
  rete totally ignore it. This is made possible by another change to the
  clock driver posted separately from this series and which this patch
  now depends on [1].

  1. [PATCH] clk: renesas: rcar-gen3: set state when registering SD clocks
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 33 +++--
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index bca6c7f51de18db7..bad062150cd486f6 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -242,6 +242,10 @@ struct sd_clock {
  *  1 0 2 (4)  0 (2)  8
  *  1 0 3 (8)  0 (2) 16
  *  1 0 4 (16) 0 (2) 32
+ *
+ *  NOTE: There is a quirk option to ignore the first row of the dividers
+ *  table when searching for suitable settings. This is because HS400 on
+ *  early ES versions of H3 and M3-W requires a specific setting to work.
  */
 static const struct sd_div_table cpg_sd_div_table[] = {
 /* CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  sd_div) */
@@ -352,6 +356,12 @@ static const struct clk_ops cpg_sd_clock_ops = {
.set_rate = cpg_sd_clock_set_rate,
 };
 
+static u32 cpg_quirks __initdata;
+
+#define PLL_ERRATA BIT(0)  /* Missing PLL0/2/4 post-divider */
+#define RCKCR_CKSELBIT(1)  /* Manual RCLK parent selection */
+#define SD_SKIP_FIRST  BIT(2)  /* Skip first clock in SD table */
+
 static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
void __iomem *base, const char *parent_name,
struct raw_notifier_head *notifiers)
@@ -377,6 +387,11 @@ static struct clk * __init cpg_sd_clk_register(const 
struct cpg_core_clk *core,
clock->div_table = cpg_sd_div_table;
clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
 
+   if (cpg_quirks & SD_SKIP_FIRST) {
+   clock->div_table++;
+   clock->div_num--;
+   }
+
val = readl(clock->csn.reg) & ~CPG_SD_FC_MASK;
val |= CPG_SD_STP_MASK | (clock->div_table[0].val & CPG_SD_FC_MASK);
writel(val, clock->csn.reg);
@@ -406,23 +421,27 @@ static struct clk * __init cpg_sd_clk_register(const 
struct cpg_core_clk *core,
 static const struct rcar_gen3_cpg_pll_config *cpg_pll_config __initdata;
 static unsigned int cpg_clk_extalr __initdata;
 static u32 cpg_mode __initdata;
-static u32 cpg_quirks __initdata;
-
-#define PLL_ERRATA BIT(0)  /* Missing PLL0/2/4 post-divider */
-#define RCKCR_CKSELBIT(1)  /* Manual RCLK parent selection */
 
 static const struct soc_device_attribute cpg_quirks_match[] __initconst = {
{
.soc_id = "r8a7795", .revision = "ES1.0",
-   .data = (void *)(PLL_ERRATA | RCKCR_CKSEL),
+   .data = (void *)(PLL_ERRATA | RCKCR_CKSEL | SD_SKIP_FIRST),
},
{
.soc_id = "r8a7795", .revision = "ES1.*",
-   .data = (void *)RCKCR_CKSEL,
+   .data = (void *)(RCKCR_CKSEL | SD_SKIP_FIRST),
+   },
+   {
+   .soc_id = "r8a7795", .revision = "ES2.0",
+   .data = (void *)SD_SKIP_FIRST,
},
{
.soc_id = "r8a7796", .revision = "ES1.0",
-   .data = (void *)RCKCR_CKSEL,
+   .data = (void *)(RCKCR_CKSEL | SD_SKIP_FIRST),
+   },
+   {
+   .soc_id = "r8a7796", .revision = "ES1.1",
+   .data = (void *)SD_SKIP_FIRST,
},
{ /* sentinel */ }
 };
-- 
2.19.1

[PATCH v2 0/2] renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Geert,

This series aims to solve the clock quirk needed to enabled HS400 on 
SoCs needing special clock handeling. It uses the same method as v1 of 
this series and which was discussed during the SDHI hackathon. However 
patch 2/2 have been completely rewritten to take your comments from v1 
into account. Due to this change this series now depends on [1].

This is tested on H3 (ES1.0, ES2.0), M3-W (ES1.0) and M3-N together with
patches to enable HS400 with great results. No regressions found for
eMMC HS200/HS400 modes nor for SDR{25,50,104} on any of the SoCs.

Patch 1/2 adds documentation on which settings is used while 2/2 is the
real change where the quirk is implemented.

1. [PATCH] clk: renesas: rcar-gen3: set state when registering SD clocks

Niklas Söderlund (2):
  clk: renesas: rcar-gen3: add documentation for SD clocks
  clk: renesas: rcar-gen3: add HS400 quirk for SD clock

 drivers/clk/renesas/rcar-gen3-cpg.c | 43 +
 1 file changed, 31 insertions(+), 12 deletions(-)

-- 
2.19.1

[PATCH] clk: renesas: rcar-gen3: set state when registering SD clocks

[Niklas Söderlund]

The driver tries to figure out which state a SD clock is in when the
clock is register instead of setting a known state. This can be
problematic for two reasons.

1. If the clock driver can't figure out the state of the clock
   registration of the clock fails and setting of a known state by a
   clock user is not possible.

2. The state of the clock depends on if and how the bootloader
   configured it. The driver only checks that the rate is known not if
   the clock is stopped or not for example.

Fix this by setting a known state and make sure the clock is stopped.

Signed-off-by: Niklas Söderlund 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index 4ba38f98cc7bab82..f6335357c85505df 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -360,7 +360,7 @@ static struct clk * __init cpg_sd_clk_register(const struct 
cpg_core_clk *core,
struct sd_clock *clock;
struct clk *clk;
unsigned int i;
-   u32 sd_fc;
+   u32 val;
 
clock = kzalloc(sizeof(*clock), GFP_KERNEL);
if (!clock)
@@ -377,17 +377,11 @@ static struct clk * __init cpg_sd_clk_register(const 
struct cpg_core_clk *core,
clock->div_table = cpg_sd_div_table;
clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
 
-   sd_fc = readl(clock->csn.reg) & CPG_SD_FC_MASK;
-   for (i = 0; i < clock->div_num; i++)
-   if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
-   break;
-
-   if (WARN_ON(i >= clock->div_num)) {
-   kfree(clock);
-   return ERR_PTR(-EINVAL);
-   }
+   val = readl(clock->csn.reg) & ~CPG_SD_FC_MASK;
+   val |= CPG_SD_STP_MASK | (clock->div_table[0].val & CPG_SD_FC_MASK);
+   writel(val, clock->csn.reg);
 
-   clock->cur_div_idx = i;
+   clock->cur_div_idx = 0;
 
clock->div_max = clock->div_table[0].div;
clock->div_min = clock->div_max;
-- 
2.19.1

Re: [PATCH v3 0/3] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

[Niklas Söderlund]

Hi Wolfram,

On 2018-11-28 23:06:37 +0100, Niklas Söderlund wrote:
> Hi Wolfram,
> 
> On 2018-11-28 22:56:20 +0100, Wolfram Sang wrote:
> > Hi Niklas,
> > 
> > thanks for the updates! Do you happen to have a branch ready for
> > testing?
> 
> I will push a new branch once I'm done updating the clock patch so all 
> changes can be tested in one go. Will let you know once that is 
> available. Hopefully later tonight.

I have pushed the latest and greatest to [1]. I tested this on H3 ES1, 
H3 ES2.0 and M3-N and it looks good. Speeds for both MMC and SD cards 
match what we saw in Edinburgh.

I still have to post the clock patches to the mailing list but in case 
you wish to test right away I thought I send this mail out before I do 
that :-)

1. git://git.ragnatech.se/linux mmc/hackfest

-- 
Regards,
Niklas Söderlund

Re: [PATCH v3 0/3] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

[Niklas Söderlund]

Hi Wolfram,

On 2018-11-28 22:56:20 +0100, Wolfram Sang wrote:
> Hi Niklas,
> 
> thanks for the updates! Do you happen to have a branch ready for
> testing?

I will push a new branch once I'm done updating the clock patch so all 
changes can be tested in one go. Will let you know once that is 
available. Hopefully later tonight.


-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2018-11-05 16:45:39 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, Nov 5, 2018 at 4:07 PM Niklas Söderlund
>  wrote:
> > On 2018-11-05 11:43:24 +0100, Geert Uytterhoeven wrote:
> > > On Thu, Nov 1, 2018 at 12:26 AM Niklas Söderlund
> > >  wrote:
> > > > From: Niklas Söderlund 
> > > >
> > > > On H3 (ES1.0,ES2.0) and M3-W (ES1.0,ES1.1) the clock setting for HS400
> > >
> > > H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1)
> >
> > Thanks.
> >
> > >
> > > > needs a quirk to function properly. The reason for the quirk is that
> > > > there are two settings which produces same divider vale for the SDn
> > > > clock. On the effected boards the one currently selected results in HS00
> > >
> > > HS200 or HS400?
> >
> > Wops, HS400 :-)
> >
> > >
> > > > not working.
> > > >
> > > > This change uses the same method as the Gen2 CPG driver and simply
> > > > ignores the first clock setting as this is the offending one when
> > > > selecting the settings. Which of the two possible settings is used have
> > > > no effect for SDR104.
> > > >
> > > > Signed-off-by: Niklas Söderlund 
> > > > ---
> > > >  drivers/clk/renesas/rcar-gen3-cpg.c | 28 ++--
> > > >  1 file changed, 22 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
> > > > b/drivers/clk/renesas/rcar-gen3-cpg.c
> > > > index ff56ac6134111c38..8cc524a29c855dd2 100644
> > > > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > > > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> 
> > > > @@ -377,6 +382,7 @@ static struct clk * __init 
> > > > cpg_sd_clk_register(const struct cpg_core_clk *core,
> > > > clock->hw.init = 
> > > > clock->div_table = cpg_sd_div_table;
> > > > clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
> > > > +   clock->skip_first = skip_first;
> > >
> > > What about
> > >
> > > if (cpg_quirks & SD_SKIP_FIRST) {
> > > clock->div_table++;
> > > clock->div_num--;
> > > }
> > >
> > > instead?
> >
> > This was my first approach as well, unfortunately it won't work :-(
> >
> > If the bootloader leaves the clock settings in a state which matches the
> > first row in the table the clock fails to register as the check in
> > cpg_sd_clk_register() makes sure it have a row for the state the
> > hardware is in. If it can't find that state the clock fails to register.
> > Whit this quirk the limitation of the effected boards only have an
> > effect when setting the clock.
> 
> IC...
> 
> > I thought this solution solved this quiet neatly with the robustness
> > principle, 'Be conservative in what you do, be liberal in what you
> > accept from others' :-)
> 
> Will it ever use the settings as inherited from boot loader or reset state?
> If it does, I assume it will fail badly, due to an inconsistency between the
> SD and SDH clock rates?
> And what if the kernel is ever booted when the SDnSRCFC or SDnFC field
> has an invalid value? Then the driver will fail, too?
> 
> Hence, isn't it safer to initialize the registers to a known safe value?

I agree with you that we should not trust the value fro the bootloader 
and the sdhi driver do not trust this and resets it. The problem is 
rcar-gen3-cpg. From cpg_sd_clk_register()

 sd_fc = readl(clock->csn.reg) & CPG_SD_FC_MASK;
 for (i = 0; i < clock->div_num; i++)
 if (sd_fc == (clock->div_table[i].val & CPG_SD_FC_MASK))
 break;

 if (WARN_ON(i >= clock->div_num)) {
 kfree(clock);
 return ERR_PTR(-EINVAL);
 }

 clock->cur_div_idx = i;

If the bootloader sets the register to a value not known by the clock 
driver or if we us the moth you prefer to modify clock->div_table and 
clock->div_num the clock driver would need to set a safe default. I 
think this would be fine as the SDHI driver could handle this (I need to 
test it tho). My proposal is therefor that I add a patch to this series 
where the clock driver try to set a known divider when it registers the 
clock.

My suggestion would be that the divider to be set to 4 as this appears 
to be what current boot loaders do. Would this work for you?


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

-- 
Regards,
Niklas Söderlund

[PATCH v3 3/3] mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.[012]

[Niklas Söderlund]

The Renesas BSP confirms that H3 ES1.x and M3-W ES1.[012] do not
properly support HS400. Add a quirk to indicate this and disable HS400
in the MMC capabilities if the quirk is set.

Signed-off-by: Niklas Söderlund 
Tested-by: Wolfram Sang 
Reviewed-by: Wolfram Sang 
Reviewed-by: Simon Horman 

---
* Changes since v2
- s/M3-W ES1.x/M3-W ES1.[012]/ in commit message and topic as HS400 is
  not disabled for M3-W ES1.3. Thanks Geert for pointing this out.
---
 drivers/mmc/host/renesas_sdhi_core.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index f20df18f49e0d7c5..d4df4e59d9f2a8ad 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -47,6 +47,7 @@
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
 struct renesas_sdhi_quirks {
+   bool hs400_disabled;
bool hs400_4taps;
 };
 
@@ -598,15 +599,21 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
-static const struct renesas_sdhi_quirks sdhi_quirks_4tap = {
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w_es1 = {
+   .hs400_disabled = true,
+   .hs400_4taps = true,
+};
+
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_es2 = {
+   .hs400_disabled = false,
.hs400_4taps = true,
 };
 
 static const struct soc_device_attribute sdhi_quirks_match[]  = {
-   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_4tap },
-   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_4tap },
-   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_4tap },
-   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_4tap },
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = 
_quirks_h3_m3w_es1 },
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_es2 
},
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = 
_quirks_h3_m3w_es1 },
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = 
_quirks_h3_m3w_es1 },
{ /* Sentinel. */ },
 };
 
@@ -695,6 +702,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_disabled)
+   host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
+
if (quirks && quirks->hs400_4taps)
mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
 
-- 
2.19.1

[PATCH v3 2/3] mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

[Niklas Söderlund]

It was though all ES revisions of H3 and M3-W SoCs required the
TMIO_MMC_HAVE_4TAP_HS400 flag. Recent datasheet updates tells us this is
not true, only early ES revisions of the SoC do.

Since quirk matching based on ES revisions is now used to handle the
flag it's possible to align all Gen3 compatibility properties. This will
allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400
mode.

Signed-off-by: Niklas Söderlund 
Tested-by: Wolfram Sang 
Reviewed-by: Wolfram Sang 
Reviewed-by: Simon Horman 
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 20 +++
 2 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 57e829223c40e0ee..332c5c60edb3d9c4 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -99,22 +99,6 @@ static const struct renesas_sdhi_of_data of_rza2_compatible 
= {
.max_segs   = 1,
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-   /* DMAC can handle 0x blk count but only 1 segment */
-   .max_blk_count  = 0x,
-   .max_segs   = 1,
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -133,8 +117,8 @@ static const struct renesas_sdhi_of_data 
of_rcar_gen3_compatible = {
 static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-r7s9210", .data = _rza2_compatible, },
{ .compatible = "renesas,sdhi-mmc-r8a77470", .data = 
_rcar_gen3_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
{},
 };
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c 
b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 1a4016f635d398c2..8471160316e073c5 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -75,19 +75,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -114,8 +101,8 @@ static const struct of_device_id 
renesas_sdhi_sys_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-r8a7792", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7793", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7794", .data = 
_rcar_gen2_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen1-sdhi", .data = 
_rcar_gen1_compatible, },
{ .compatible = "renesas,rcar-gen2-sdhi", .data = 
_rcar_gen2_compatible, },
{ .compatible = &

[PATCH v3 1/3] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

[Niklas Söderlund]

Latest datasheet makes it clear that not all ES revisions of the H3 and
M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
unconditionally for these two SoCs. Prepare to handle the quirk based on
SoC revision instead of compatibility value by using soc_device_match()
and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.

The reason for adding a new quirks struct instead of just a flag is that
looking ahead it seems more quirks needs to be handled in a SoC revision
basis.

Signed-off-by: Niklas Söderlund 
Tested-by: Wolfram Sang 
Reviewed-by: Wolfram Sang 
Reviewed-by: Simon Horman 

---
* Changes since v2
- Renamed sdhi_quirks_h3_m3w to sdhi_quirks_4tap. This have little
  effect as the last patch in this series renames the variable once more
  once more quirks are added which are more SoC specific. Suggested by
  Geert, thanks.
---
 drivers/mmc/host/renesas_sdhi_core.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 78bd117bbe65de46..f20df18f49e0d7c5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "renesas_sdhi.h"
 #include "tmio_mmc.h"
@@ -45,6 +46,10 @@
 #define SDHI_VER_GEN3_SD   0xcc10
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
+struct renesas_sdhi_quirks {
+   bool hs400_4taps;
+};
+
 static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 {
u32 val;
@@ -593,11 +598,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const struct renesas_sdhi_quirks sdhi_quirks_4tap = {
+   .hs400_4taps = true,
+};
+
+static const struct soc_device_attribute sdhi_quirks_match[]  = {
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_4tap },
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_4tap },
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_4tap },
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_4tap },
+   { /* Sentinel. */ },
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
   const struct tmio_mmc_dma_ops *dma_ops)
 {
struct tmio_mmc_data *mmd = pdev->dev.platform_data;
+   const struct renesas_sdhi_quirks *quirks = NULL;
const struct renesas_sdhi_of_data *of_data;
+   const struct soc_device_attribute *attr;
struct tmio_mmc_data *mmc_data;
struct tmio_mmc_dma *dma_priv;
struct tmio_mmc_host *host;
@@ -607,6 +626,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
of_data = of_device_get_match_data(>dev);
 
+   attr = soc_device_match(sdhi_quirks_match);
+   if (attr)
+   quirks = attr->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
@@ -672,6 +695,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_4taps)
+   mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
+
/* For some SoC, we disable internal WP. GPIO may override this */
if (mmc_can_gpio_ro(host->mmc))
mmc_data->capabilities2 &= ~MMC_CAP2_NO_WRITE_PROTECT;
-- 
2.19.1

[PATCH v3 0/3] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

[Niklas Söderlund]

Hi,

Recent datasheet updates have made it clear that some quirks are not SoC
specific but SoC + ES version specific. Currently the quirks are
selected using compatibility values but whit this new information that
is not enough.

Patch 1/3 adds support to select quirks based on SoC + ES revision using
soc_device_match() and converts the only existing quirk. Patch 2/3
Removes the old method to select quirk based on compatibility string.
While Patch 3/3 adds a new quirk from the BSP which blacklists some
known problematic ES versions for HS400. HS400 is not yet enabled
upstream so blacklisting these ES versions is not a regression of
functionality.

Based on mmc/next and tested on H3 and M3-N.

Niklas Söderlund (3):
  mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
  mmc: renesas_sdhi: align compatibility properties for H3 and M3-W
  mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.[012]

 drivers/mmc/host/renesas_sdhi_core.c  | 36 +++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 20 ++-
 3 files changed, 41 insertions(+), 35 deletions(-)

-- 
2.19.1

[PATCH v4 3/3] mmc: renesas_sdhi: add initial setting of interrupt mask register

[Niklas Söderlund]

The initial value of the interrupt mask register may be different from
the H/W manual at the startup of the kernel by setting from the
bootloader. Since the error interrupts may be unmasked, the driver sets
initial value.

The initial value is only known for R-Car Gen2 and Gen3 platforms so
limit the initialization to those platforms.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 
Tested-by: Wolfram Sang 
Reviewed-by: Wolfram Sang 
Reviewed-by: Simon Horman 

---

* Changes since v1
- Limit the initialization to Gen2+ platforms by checking the
  TMIO_MMC_MIN_RCAR2 flag.
- Rename the constant for the initialization value to reflect it's only
  for R-Car Gen2+ platforms.
- Move setting of the initial mask to renesas_sdhi.
---
 drivers/mmc/host/renesas_sdhi_core.c | 4 
 drivers/mmc/host/tmio_mmc.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 78bd117bbe65de46..26da095f8ad5a56c 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -523,6 +523,10 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host 
*host)
sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
   ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
   sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+   if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
+   sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK,
+TMIO_MASK_INIT_RCAR2);
 }
 
 static int renesas_sdhi_wait_idle(struct tmio_mmc_host *host, u32 bit)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1e317027bf534612..5f6dfb86e43e8208 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -96,6 +96,7 @@
 
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
+#define TMIO_MASK_INIT_RCAR2   0x8b7f031d /* Initial value for R-Car Gen2+ */
 #define TMIO_MASK_ALL   0x837f031d
 #define TMIO_MASK_READOP  (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND)
 #define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND)
-- 
2.19.1

[PATCH v4 0/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

Hi,

While looking at the Renesas BSP kernel I found patches which improves
the state of the hardware at probe and after runtime resume.

Patch 1/3 make sure the module clock is enabled after resuming before
register are accessed. Patch 2/3 is the real change in this series and
brings in reset of the vendor specific callback when resetting (SCC in
the Renesas case). While 3/3 simply make sure that the IRQ mask for
Renesas boards (Gen2 and later) are in a good shape after probe (and
reset).

In addition to addressing the state after resuming it helped unbreak a
SD card I have which are very problematic on Koelsch. Without this
series inserting the card results in:

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: Tuning procedure failed
mmc0: tuning execution failed: -5
mmc0: error -5 whilst initialising SD card
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)

While with this series applied (patch 2/3):

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
mmc0: new ultra high speed SDR50 SDHC card at address 
mmcblk0: mmc0: SU04G 3.69 GiB
 mmcblk0: p1 p2

Patch 1/3 was previously part of 2/3 but as it deals with a unrelated
issue it's here broken out to a separate patch. Patch 3/3 have once been
posted outside this series bit after comments from Wolfram it's brought
back as it now deepens on 2/3.

Most changes in this series are based on similar work from Masaharu
Hayakawa but for this version all changes now differ quiet a lot from
his work.  All mails sent to him bounce with a "Undelivered Mail
Returned to Sender" error. I therefor felt the need to claim authorship
as I don't want to post blame of my (potential) mistakes on some else.

Niklas Söderlund (3):
  mmc: tmio: enable module clock before resetting when resuming
  mmc: tmio: fix reset operation
  mmc: renesas_sdhi: add initial setting of interrupt mask register

 drivers/mmc/host/renesas_sdhi_core.c |  4 
 drivers/mmc/host/tmio_mmc.h  |  1 +
 drivers/mmc/host/tmio_mmc_core.c | 27 +++
 3 files changed, 20 insertions(+), 12 deletions(-)

-- 
2.19.1

[PATCH v4 1/3] mmc: tmio: enable module clock before resetting when resuming

[Niklas Söderlund]

On runtime power management resume, the host clock needs to be
enabled before calling tmio_mmc_reset. If the mmc device has a power
domain entry, the host clock is enabled via genpd_runtime_resume,
running before tmio_mmc_host_runtime_resume. If the mmc device has no
power domain entry, however, genpd_runtime_resume is not called. This
patch changes tmio_mmc_host_runtime_resume to enable the host clock
before calling tmio_mmc_reset.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 
Tested-by: Wolfram Sang 
Reviewed-by: Wolfram Sang 
Reviewed-by: Masahiro Yamada 
Reviewed-by: Simon Horman 
---
 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index a8f917f744fb9f63..35acfa4f40b2ef57 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1326,8 +1326,8 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 {
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
-   host->reset(host);
tmio_mmc_clk_enable(host);
+   host->reset(host);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

[PATCH v4 2/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

SD / MMC did not operate properly when suspend transition failed.
Because the SCC was not reset at resume, issue of the command failed.
Call the host specific reset function and reset the hardware in order to
add reset of SCC. This change also fixes tuning on some stubborn cards
on Gen2.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 

---
* Changes sine v1
- Merge tmio_mmc_reset() into tmio_mmc_hw_reset() as it's now the only
  caller.

* Changes since v2
- Rebased on mmc/next caused small refactoring of the code.

* Changes since v3
- Remove call to tmio_mmc_abort_dma() in tmio_mmc_reset_work() as it
  already calls tmio_mmc_hw_reset() which with this change already
  aborts the DMA. Thanks Yamada-san for pointing this out.
---
 drivers/mmc/host/tmio_mmc_core.c | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 35acfa4f40b2ef57..d396c5156053e410 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -171,6 +171,18 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
}
 }
 
+static void tmio_mmc_hw_reset(struct mmc_host *mmc)
+{
+   struct tmio_mmc_host *host = mmc_priv(mmc);
+
+   host->reset(host);
+
+   tmio_mmc_abort_dma(host);
+
+   if (host->hw_reset)
+   host->hw_reset(host);
+}
+
 static void tmio_mmc_reset_work(struct work_struct *work)
 {
struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host,
@@ -209,12 +221,11 @@ static void tmio_mmc_reset_work(struct work_struct *work)
 
spin_unlock_irqrestore(>lock, flags);
 
-   host->reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
/* Ready for new calls */
host->mrq = NULL;
 
-   tmio_mmc_abort_dma(host);
mmc_request_done(host->mmc, mrq);
 }
 
@@ -696,14 +707,6 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
return 0;
 }
 
-static void tmio_mmc_hw_reset(struct mmc_host *mmc)
-{
-   struct tmio_mmc_host *host = mmc_priv(mmc);
-
-   if (host->hw_reset)
-   host->hw_reset(host);
-}
-
 static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
struct tmio_mmc_host *host = mmc_priv(mmc);
@@ -1226,7 +1229,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 
_host->set_clock(_host, 0);
-   _host->reset(_host);
+   tmio_mmc_hw_reset(mmc);
 
_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
CTL_IRQ_MASK);
tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
@@ -1327,7 +1330,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
tmio_mmc_clk_enable(host);
-   host->reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

Re: [PATCH v3 2/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

Hi Yamada-san,

Thanks for your feedback.

On 2018-11-02 15:54:17 +0900, Masahiro Yamada wrote:
> On Thu, Nov 1, 2018 at 8:53 AM Niklas Söderlund
>  wrote:
> >
> > From: Niklas Söderlund 
> >
> > SD / MMC did not operate properly when suspend transition failed.
> > Because the SCC was not reset at resume, issue of the command failed.
> > Call the host specific reset function and reset the hardware in order to
> > add reset of SCC. This change also fixes tuning on some stubborn cards
> > on Gen2.
> >
> > Based on work from Masaharu Hayakawa.
> >
> > Signed-off-by: Niklas Söderlund 
> >
> > ---
> > * Changes sine v1
> > - Merge tmio_mmc_reset() into tmio_mmc_hw_reset() as it's now the only
> >   caller.
> >
> > * Changes since v2
> > - Rebased on mmc/next caused small refactoring of the code.
> > ---
> >  drivers/mmc/host/tmio_mmc_core.c | 26 +++---
> >  1 file changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/mmc/host/tmio_mmc_core.c 
> > b/drivers/mmc/host/tmio_mmc_core.c
> > index 953562a12a0d6ebc..662161be03b6d52e 100644
> > --- a/drivers/mmc/host/tmio_mmc_core.c
> > +++ b/drivers/mmc/host/tmio_mmc_core.c
> > @@ -171,6 +171,18 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
> > }
> >  }
> >
> > +static void tmio_mmc_hw_reset(struct mmc_host *mmc)
> > +{
> > +   struct tmio_mmc_host *host = mmc_priv(mmc);
> > +
> > +   host->reset(host);
> > +
> > +   tmio_mmc_abort_dma(host);
> > +
> > +   if (host->hw_reset)
> > +   host->hw_reset(host);
> > +}
> > +
> >  static void tmio_mmc_reset_work(struct work_struct *work)
> >  {
> > struct tmio_mmc_host *host = container_of(work, struct 
> > tmio_mmc_host,
> > @@ -209,7 +221,7 @@ static void tmio_mmc_reset_work(struct work_struct 
> > *work)
> >
> > spin_unlock_irqrestore(>lock, flags);
> >
> > -   host->reset(host);
> > +   tmio_mmc_hw_reset(host->mmc);
> >
> > /* Ready for new calls */
> > host->mrq = NULL;
> 
> 
> 
> I see tmio_mmc_abort_dma() a few lines below.
> 
> If you add tmio_mmc_abort_dma() into tmio_mmc_hw_reset(),
> you do not need to abort DMA twice, don't you?
> 
> 
> 
> 
> tmio_mmc_hw_reset(host->mmc);
> 
> /* Ready for new calls */
> host->mrq = NULL;
> 
> tmio_mmc_abort_dma(host);  /* <-- abort DMA again? */
> mmc_request_done(host->mmc, mrq);
> }
> 

You are correct with this change the call to tmio_mmc_abort_dma() can be 
dropped here. Will do so and send out a new version. Thanks for pointing 
this out!

> 
> > @@ -696,14 +708,6 @@ static int tmio_mmc_start_data(struct tmio_mmc_host 
> > *host,
> > return 0;
> >  }
> >
> > -static void tmio_mmc_hw_reset(struct mmc_host *mmc)
> > -{
> > -   struct tmio_mmc_host *host = mmc_priv(mmc);
> > -
> > -   if (host->hw_reset)
> > -   host->hw_reset(host);
> > -}
> > -
> >  static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> >  {
> > struct tmio_mmc_host *host = mmc_priv(mmc);
> > @@ -1228,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
> > _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> >
> > _host->set_clock(_host, 0);
> > -   _host->reset(_host);
> > +   tmio_mmc_hw_reset(mmc);
> 
> 
> I think it is weird to call tmio_mmc_abort_dma()
> before tmio_mmc_request_dma().
> 
> 
> 
> 
> 
> > _host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
> > CTL_IRQ_MASK);
> > tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
> > @@ -1329,7 +1333,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
> > struct tmio_mmc_host *host = dev_get_drvdata(dev);
> >
> > tmio_mmc_clk_enable(host);
> > -   host->reset(host);
> > +   tmio_mmc_hw_reset(host->mmc);
> >
> > if (host->clk_cache)
> > host->set_clock(host, host->clk_cache);
> > --
> > 2.19.1
> >
> 
> 
> --
> Best Regards
> Masahiro Yamada

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] mmc: core: remove obsolete mmc_set_blockcount() function

[Niklas Söderlund]

Hi Wolfram,

Thanks for your patch.

On 2018-11-26 14:38:14 +0100, Wolfram Sang wrote:
> The only user was converted to fill a sbc command which is the proper
> way to do it because of AutoCMD23 feature of some hosts.
> 
> Signed-off-by: Wolfram Sang 
> Tested-by: Clément Péron 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/mmc/core/core.c | 14 --
>  drivers/mmc/core/core.h |  2 --
>  2 files changed, 16 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 50a5c340307b..d3085f70e9a4 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2413,20 +2413,6 @@ int mmc_set_blocklen(struct mmc_card *card, unsigned 
> int blocklen)
>  }
>  EXPORT_SYMBOL(mmc_set_blocklen);
>  
> -int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
> - bool is_rel_write)
> -{
> - struct mmc_command cmd = {};
> -
> - cmd.opcode = MMC_SET_BLOCK_COUNT;
> - cmd.arg = blockcount & 0x;
> - if (is_rel_write)
> - cmd.arg |= 1 << 31;
> - cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC;
> - return mmc_wait_for_cmd(card->host, , 5);
> -}
> -EXPORT_SYMBOL(mmc_set_blockcount);
> -
>  static void mmc_hw_reset_for_init(struct mmc_host *host)
>  {
>   mmc_pwrseq_reset(host);
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 087ba68b2920..8fb6bc37f808 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -118,8 +118,6 @@ int mmc_erase_group_aligned(struct mmc_card *card, 
> unsigned int from,
>  unsigned int mmc_calc_max_discard(struct mmc_card *card);
>  
>  int mmc_set_blocklen(struct mmc_card *card, unsigned int blocklen);
> -int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
> - bool is_rel_write);
>  
>  int __mmc_claim_host(struct mmc_host *host, struct mmc_ctx *ctx,
>atomic_t *abort);
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] mmc: core: use mrq->sbc when sending CMD23 for RPMB

[Niklas Söderlund]

Hi Wolfram,

Thanks for your patch.

On 2018-11-26 14:38:13 +0100, Wolfram Sang wrote:
> When sending out CMD23 in the blk preparation, the comment there
> rightfully says:
> 
>* However, it is not sufficient to just send CMD23,
>* and avoid the final CMD12, as on an error condition
>* CMD12 (stop) needs to be sent anyway. This, coupled
>* with Auto-CMD23 enhancements provided by some
>* hosts, means that the complexity of dealing
>* with this is best left to the host. If CMD23 is
>* supported by card and host, we'll fill sbc in and let
>* the host deal with handling it correctly.
> 
> Let's do this behaviour for RPMB as well, and not send CMD23
> independently. Otherwise IP cores (like Renesas SDHI) may timeout
> because of automatic CMD23/CMD12 handling.
> 
> Reported-by: Masaharu Hayakawa 
> Signed-off-by: Wolfram Sang 
> Tested-by: Clément Péron 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/mmc/core/block.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index c35b5b08bb33..111934838da2 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -472,7 +472,7 @@ static int ioctl_do_sanitize(struct mmc_card *card)
>  static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data 
> *md,
>  struct mmc_blk_ioc_data *idata)
>  {
> - struct mmc_command cmd = {};
> + struct mmc_command cmd = {}, sbc = {};
>   struct mmc_data data = {};
>   struct mmc_request mrq = {};
>   struct scatterlist sg;
> @@ -550,10 +550,15 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, 
> struct mmc_blk_data *md,
>   }
>  
>   if (idata->rpmb) {
> - err = mmc_set_blockcount(card, data.blocks,
> - idata->ic.write_flag & (1 << 31));
> - if (err)
> - return err;
> + sbc.opcode = MMC_SET_BLOCK_COUNT;
> + /*
> +  * We don't do any blockcount validation because the max size
> +  * may be increased by a future standard. We just copy the
> +  * 'Reliable Write' bit here.
> +  */
> + sbc.arg = data.blocks | (idata->ic.write_flag & BIT(31));
> + sbc.flags = MMC_RSP_R1 | MMC_CMD_AC;
> + mrq.sbc = 
>   }
>  
>   if ((MMC_EXTRACT_INDEX_FROM_ARG(cmd.arg) == EXT_CSD_SANITIZE_START) &&
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH] mmc: tmio: introduce mask for 'always 1' bits

[Niklas Söderlund]

Hi Wolfram,

Thanks for your work.

On 2018-11-23 13:57:32 +0100, Simon Horman wrote:
> On Thu, Nov 22, 2018 at 03:06:59PM +0100, Wolfram Sang wrote:
> > 
> > > > +#define TMIO_STAT_ALWAYS_SET_27BIT(27) /* only known on R-Car 
> > > > 2+ so far */
> > > 
> > > The _27 seems to be odd to include in the name, but I assume it
> > > was the least bad option you could come up with.
> > 
> > Yes :) I am open for better suggestions.
> 
> Simply dropping '_27' would be an improvement in my opinion.

I'm slightly agreeing with Simon here to drop the _27. With or without 
that change I think this patch looks good so feel free to add.

Reviewed-by: Niklas Söderlund 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v3 0/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

Hi Ulf, Wolfram

On 2018-11-19 14:33:58 +0100, Wolfram Sang wrote:
> 
> > Sure, no problem. I drop this and the other series then.
> 
> Thanks, Ulf!
> 

Thanks for looking out for me here, I will fly home tomorrow so I hope 
to get a new versions of these series out late this week.

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2018-11-05 11:43:24 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> Thanks for your patch!
> 
> On Thu, Nov 1, 2018 at 12:26 AM Niklas Söderlund
>  wrote:
> > From: Niklas Söderlund 
> >
> > On H3 (ES1.0,ES2.0) and M3-W (ES1.0,ES1.1) the clock setting for HS400
> 
> H3 (ES1.x, ES2.0) and M3-W (ES1.0, ES1.1)

Thanks.

> 
> > needs a quirk to function properly. The reason for the quirk is that
> > there are two settings which produces same divider vale for the SDn
> > clock. On the effected boards the one currently selected results in HS00
> 
> HS200 or HS400?

Wops, HS400 :-)

> 
> > not working.
> >
> > This change uses the same method as the Gen2 CPG driver and simply
> > ignores the first clock setting as this is the offending one when
> > selecting the settings. Which of the two possible settings is used have
> > no effect for SDR104.
> >
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/clk/renesas/rcar-gen3-cpg.c | 28 ++--
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
> > b/drivers/clk/renesas/rcar-gen3-cpg.c
> > index ff56ac6134111c38..8cc524a29c855dd2 100644
> > --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> > @@ -227,6 +227,7 @@ struct sd_clock {
> > unsigned int div_min;
> > unsigned int div_max;
> > unsigned int cur_div_idx;
> > +   bool skip_first;
> 
> I think you can do without adding this field...
> 
> >  };
> >
> >  /* SDn divider
> > @@ -243,6 +244,10 @@ struct sd_clock {
> >   *  1 0 2 (4)  0 (2)  8
> >   *  1 0 3 (8)  0 (2) 16
> >   *  1 0 4 (16) 0 (2) 32
> > + *
> > + *  NOTE: There is a quirk option to ignore the first row of the dividers
> > + *  table when searching for suitable settings. This is because HS400 on
> > + *  early ES versions of H3 and M3-W requires a specific setting to work.
> >   */
> >  static const struct sd_div_table cpg_sd_div_table[] = {
> >  /* CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  
> > sd_div) */
> > @@ -327,7 +332,7 @@ static int cpg_sd_clock_set_rate(struct clk_hw *hw, 
> > unsigned long rate,
> > u32 val;
> > unsigned int i;
> >
> > -   for (i = 0; i < clock->div_num; i++)
> > +   for (i = clock->skip_first ? 1 : 0; i < clock->div_num; i++)
> 
> ... and without this change (see below)
> 
> > if (div == clock->div_table[i].div)
> > break;
> >
> > @@ -355,7 +360,7 @@ static const struct clk_ops cpg_sd_clock_ops = {
> >
> >  static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk 
> > *core,
> > void __iomem *base, const char *parent_name,
> > -   struct raw_notifier_head *notifiers)
> > +   struct raw_notifier_head *notifiers, bool skip_first)
> 
> I think you can do without adding this parameter.
> 
> >  {
> > struct clk_init_data init;
> > struct sd_clock *clock;
> > @@ -377,6 +382,7 @@ static struct clk * __init cpg_sd_clk_register(const 
> > struct cpg_core_clk *core,
> > clock->hw.init = 
> > clock->div_table = cpg_sd_div_table;
> > clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
> > +   clock->skip_first = skip_first;
> 
> What about
> 
> if (cpg_quirks & SD_SKIP_FIRST) {
> clock->div_table++;
> clock->div_num--;
> }
> 
> instead?

This was my first approach as well, unfortunately it won't work :-(

If the bootloader leaves the clock settings in a state which matches the 
first row in the table the clock fails to register as the check in 
cpg_sd_clk_register() makes sure it have a row for the state the 
hardware is in. If it can't find that state the clock fails to register. 
Whit this quirk the limitation of the effected boards only have an 
effect when setting the clock.

I thought this solution solved this quiet neatly with the robustness 
principle, 'Be conservative in what you do, be liberal in what you 
accept from others' :-)

I will wait for your feedback on this before sending a updated version 
of this series addressing the other comments.

> 
> It does require moving cpg_quirks and the quirk definitions up, but reduces
> the actual code change, and makes the code easier to follow.
> 
> >
> &g

Re: [PATCH 0/2] renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2018-11-05 11:32:15 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Thu, Nov 1, 2018 at 12:26 AM Niklas Söderlund
>  wrote:
> > This is the result of the SDHI hackathon for a possible solution to the
> > clock issue on early ES versions. It is based on the Gen2 solution where
> > a row of the possible clock settings are ignored on the effected SoC+ES
> > versions. The first row is not effected when reading settings left by
> > the bootloader, only when the setting the clock.
> 
> To clarify, you decided not to describe the SDH clock in DT, but went for
> heuristics with a quirk instead?
> 
> Is there any chance this can start to bite us in the future?

We discussed this at the SDHI hackfest and we could not think of any 
future situation where this could come back and bite us.

> 
> 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

-- 
Regards,
Niklas Söderlund

Re: [v2 PATCH] thermal: rcar_gen3_thermal: Fix init value of IRQCTL register

[Niklas Söderlund]

Hi Hoan-san,

Thanks for your patch.

On 2018-10-25 11:13:41 +0900, Nguyen An Hoan wrote:
> From: Hoan Nguyen An 
> 
> Fix setting value for IRQCTL register. We are setting the last 6 bits
> of (IRQCTL) to be 1 (0x3f), this is only suitable for H3ES1.*, according
> to new Hardware manual values 1 are "setting prohibited" for Gen3!

Hum, as you point out this change is not suitable for H3 ES1.x so this 
would introduce a regression, which is not good. I will leave the final 
word to Wolfram but in my view you need to introduce quirk handling to 
keep support for H3 ES1.x or blacklist that SoC in the driver.

> 
> Signed-off-by: Hoan Nguyen An 
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c 
> b/drivers/thermal/rcar_gen3_thermal.c
> index 7aed533..fde3fd8 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -306,7 +306,7 @@ static void rcar_gen3_thermal_init(struct 
> rcar_gen3_thermal_tsc *tsc)
>  
>   usleep_range(1000, 2000);
>  
> - rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);
>   rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
>   rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH] mmc: renesas_sdhi: remove workaround for HS400 clock

[Niklas Söderlund]

Hi Simon,

On 2018-11-02 12:55:02 +0100, Simon Horman wrote:
> On Wed, Oct 31, 2018 at 11:59:44PM +0100, Niklas Söderlund wrote:
> > From: Niklas Söderlund 
> > 
> > The driver sets an incorrect clock and depends on the clock driver
> > knowledge of this incorrect setting to still set a 200Mhz SDn clock.
> > Instead of spreading the workaround between the two drivers the clock
> > driver should be made aware of the ES versions where the special clock
> > handling is needed no need to keep this workaround in the SDHI driver.
> > 
> > Signed-off-by: Niklas Söderlund 
> 
> Does this change cause a regression pending an update
> to the clock driver?

No it does not, the corresponding BSP commit to the clock driver [1] was 
never part of upstream. This change should never have been merged 
upstream as it uses the hack from the BSP clock driver. Also HS400 have 
never been enabled upstream for Gen3.

1. 11fca067bde0221d ("clk: renesas: rcar-gen3: Fix SD divider setting")

-- 
Regards,
Niklas Söderlund

Re: [PATCH] thermal: rcar_gen3_thermal: Add Standby-Mode function support

[Niklas Söderlund]

Hi Hoan,

Thanks for your work.

On 2018-11-02 16:06:10 +0900, Nguyen An Hoan wrote:
> From: Hoan Nguyen An 
> 
> According to the hardware manual, Gen3 supports Standby-mode.
> Add this function, and we should use this function while
> suspend to reduce the energy consumption.

Out of curiosity is the power saved measurable?

> 
> Signed-off-by: Hoan Nguyen An 
> ---
>  drivers/thermal/rcar_gen3_thermal.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c 
> b/drivers/thermal/rcar_gen3_thermal.c
> index 7aed533..85fc4b2 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -447,11 +447,32 @@ static int rcar_gen3_thermal_probe(struct 
> platform_device *pdev)
>   return ret;
>  }
>  
> +static int rcar_gen3_thermal_standby(struct rcar_gen3_thermal_priv* priv)

I don't want to bikeshed about the name but the datasheet confuses me as 
it documents the standby and reset procedures to be exactly the same, or 
am I missing something? If they are indeed one and the same I would 
prefers this function to be called rcar_gen3_thermal_reset() and in 
rcar_gen3_thermal_suspend() add a comment "Reset to enter standby mode" 
or something similar. But it's up to you.

> +{
> + unsigned int i;
> + u32 reg_val;
> +
> + for (i = 0; i < TSC_MAX_NUM; i++) {
> + struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
> +
> + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, 0);
> + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
> +
> + reg_val = rcar_gen3_thermal_read(tsc, REG_GEN3_THCTR);
> + rcar_gen3_thermal_write(tsc, REG_GEN3_THCTR, reg_val & 
> ~THCTR_THSST);

Too many new lines.

> +
> +
> + usleep_range(1000, 2000);

Can't the sleep be moved outside the loop? We can reset all TSC and then 
wait the required 1 ms only once while they all are reset or are they 
somehow dependent on being reset in sequence?

> + }
> +
> + return 0;
> +}
> +
>  static int __maybe_unused rcar_gen3_thermal_suspend(struct device *dev)
>  {
>   struct rcar_gen3_thermal_priv *priv = dev_get_drvdata(dev);
>  
> - rcar_thermal_irq_set(priv, false);
> + rcar_gen3_thermal_standby(priv);
>  
>   return 0;
>  }
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH] thermal: rcar_gen3_thermal: Fix does not have interrupts counting

[Niklas Söderlund]

>  
>   spin_lock_irqsave(>lock, flags);
>   rcar_thermal_irq_set(priv, true);
> @@ -306,7 +311,7 @@ static void rcar_gen3_thermal_init(struct 
> rcar_gen3_thermal_tsc *tsc)
>  
>   usleep_range(1000, 2000);
>  
> - rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0x3F);
> + rcar_gen3_thermal_write(tsc, REG_GEN3_IRQCTL, 0);

This looks correct according to the new datasheets but should be posted 
in a separate patch.

>   rcar_gen3_thermal_write(tsc, REG_GEN3_IRQMSK, 0);
>   rcar_gen3_thermal_write(tsc, REG_GEN3_IRQEN, IRQ_TEMPD1 | IRQ_TEMP2);
>  
> @@ -414,6 +419,8 @@ static int rcar_gen3_thermal_probe(struct platform_device 
> *pdev)
>   priv->thermal_init(tsc);
>   rcar_gen3_thermal_calc_coefs(>coef, ptat, thcode[i]);
>  
> + rcar_gen3_thermal_update_threshold(tsc);
> +
>   zone = devm_thermal_zone_of_sensor_register(dev, i, tsc,
>   
> _gen3_tz_of_ops);
>   if (IS_ERR(zone)) {
> @@ -465,7 +472,7 @@ static int __maybe_unused rcar_gen3_thermal_resume(struct 
> device *dev)
>   struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i];
>  
>   priv->thermal_init(tsc);
> - rcar_gen3_thermal_set_trips(tsc, tsc->low, tsc->high);
> + rcar_gen3_thermal_update_threshold(tsc);
>   }
>  
>   rcar_thermal_irq_set(priv, true);
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

[PATCH v3 2/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

From: Niklas Söderlund 

SD / MMC did not operate properly when suspend transition failed.
Because the SCC was not reset at resume, issue of the command failed.
Call the host specific reset function and reset the hardware in order to
add reset of SCC. This change also fixes tuning on some stubborn cards
on Gen2.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 

---
* Changes sine v1
- Merge tmio_mmc_reset() into tmio_mmc_hw_reset() as it's now the only
  caller.

* Changes since v2
- Rebased on mmc/next caused small refactoring of the code.
---
 drivers/mmc/host/tmio_mmc_core.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 953562a12a0d6ebc..662161be03b6d52e 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -171,6 +171,18 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
}
 }
 
+static void tmio_mmc_hw_reset(struct mmc_host *mmc)
+{
+   struct tmio_mmc_host *host = mmc_priv(mmc);
+
+   host->reset(host);
+
+   tmio_mmc_abort_dma(host);
+
+   if (host->hw_reset)
+   host->hw_reset(host);
+}
+
 static void tmio_mmc_reset_work(struct work_struct *work)
 {
struct tmio_mmc_host *host = container_of(work, struct tmio_mmc_host,
@@ -209,7 +221,7 @@ static void tmio_mmc_reset_work(struct work_struct *work)
 
spin_unlock_irqrestore(>lock, flags);
 
-   host->reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
/* Ready for new calls */
host->mrq = NULL;
@@ -696,14 +708,6 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
return 0;
 }
 
-static void tmio_mmc_hw_reset(struct mmc_host *mmc)
-{
-   struct tmio_mmc_host *host = mmc_priv(mmc);
-
-   if (host->hw_reset)
-   host->hw_reset(host);
-}
-
 static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
struct tmio_mmc_host *host = mmc_priv(mmc);
@@ -1228,7 +1232,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 
_host->set_clock(_host, 0);
-   _host->reset(_host);
+   tmio_mmc_hw_reset(mmc);
 
_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
CTL_IRQ_MASK);
tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
@@ -1329,7 +1333,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
tmio_mmc_clk_enable(host);
-   host->reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

[PATCH v3 0/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

From: Niklas Söderlund 

Hi,

While looking at the Renesas BSP kernel I found patches which improves
the state of the hardware at probe and after runtime resume.

Patch 1/3 make sure the module clock is enabled after resuming before
register are accessed. Patch 2/3 is the real change in this series and
brings in reset of the vendor specific callback when resetting (SCC in
the Renesas case). While 3/3 simply make sure that the IRQ mask for
Renesas boards (Gen2 and later) are in a good shape after probe (and
reset).

In addition to addressing the state after resuming it helped unbreak a
SD card I have which are very problematic on Koelsch. Without this
series inserting the card results in:

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: Tuning procedure failed
mmc0: tuning execution failed: -5
mmc0: error -5 whilst initialising SD card
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)

While with this series applied (patch 2/3):

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
mmc0: new ultra high speed SDR50 SDHC card at address 
mmcblk0: mmc0: SU04G 3.69 GiB
 mmcblk0: p1 p2

Patch 1/3 was previously part of 2/3 but as it deals with a unrelated
issue it's here broken out to a separate patch. Patch 3/3 have once been
posted outside this series bit after comments from Wolfram it's brought
back as it now deepens on 2/3.

Most changes in this series are based on similar work from Masaharu
Hayakawa but for this version all changes now differ quiet a lot from
his work.  All mails sent to him bounce with a "Undelivered Mail
Returned to Sender" error. I therefor felt the need to claim authorship
as I don't want to post blame of my (potential) mistakes on some else.

Niklas Söderlund (3):
  mmc: tmio: enable module clock before resetting when resuming
  mmc: tmio: fix reset operation
  mmc: renesas_sdhi: add initial setting of interrupt mask register

 drivers/mmc/host/renesas_sdhi_core.c |  4 
 drivers/mmc/host/tmio_mmc.h  |  1 +
 drivers/mmc/host/tmio_mmc_core.c | 26 +++---
 3 files changed, 20 insertions(+), 11 deletions(-)

-- 
2.19.1

[PATCH v3 1/3] mmc: tmio: enable module clock before resetting when resuming

[Niklas Söderlund]

From: Niklas Söderlund 

On runtime power management resume, the host clock needs to be
enabled before calling tmio_mmc_reset. If the mmc device has a power
domain entry, the host clock is enabled via genpd_runtime_resume,
running before tmio_mmc_host_runtime_resume. If the mmc device has no
power domain entry, however, genpd_runtime_resume is not called. This
patch changes tmio_mmc_host_runtime_resume to enable the host clock
before calling tmio_mmc_reset.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 8d64f6196f33e882..953562a12a0d6ebc 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1328,8 +1328,8 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 {
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
-   host->reset(host);
tmio_mmc_clk_enable(host);
+   host->reset(host);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

[PATCH v3 3/3] mmc: renesas_sdhi: add initial setting of interrupt mask register

[Niklas Söderlund]

From: Niklas Söderlund 

The initial value of the interrupt mask register may be different from
the H/W manual at the startup of the kernel by setting from the
bootloader. Since the error interrupts may be unmasked, the driver sets
initial value.

The initial value is only known for R-Car Gen2 and Gen3 platforms so
limit the initialization to those platforms.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 

---

* Changes since v1
- Limit the initialization to Gen2+ platforms by checking the
  TMIO_MMC_MIN_RCAR2 flag.
- Rename the constant for the initialization value to reflect it's only
  for R-Car Gen2+ platforms.
- Move setting of the initial mask to renesas_sdhi.
---
 drivers/mmc/host/renesas_sdhi_core.c | 4 
 drivers/mmc/host/tmio_mmc.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index d3ac43c3d0b655dc..f2162f2b7de3ae05 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -532,6 +532,10 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host 
*host)
sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
   ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
   sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+   if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
+   sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK,
+TMIO_MASK_INIT_RCAR2);
 }
 
 static int renesas_sdhi_wait_idle(struct tmio_mmc_host *host, u32 bit)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 1e317027bf534612..5f6dfb86e43e8208 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -96,6 +96,7 @@
 
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
+#define TMIO_MASK_INIT_RCAR2   0x8b7f031d /* Initial value for R-Car Gen2+ */
 #define TMIO_MASK_ALL   0x837f031d
 #define TMIO_MASK_READOP  (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND)
 #define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND)
-- 
2.19.1

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

[Niklas Söderlund]

From: Niklas Söderlund 

Successfully tested on H3 ES2.0 and M3-N ES1.0.
Transfer rates where >160MB/s for H3 and >200MB/s for M3-N.

Signed-off-by: Niklas Söderlund 
---
 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 1 +
 arch/arm64/boot/dts/renesas/ulcb.dtsi| 1 +
 2 files changed, 2 insertions(+)
---

Hi Simon,

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.

- [PATCH] mmc: renesas_sdhi: remove workaround for HS400 clock
- [PATCH v3 0/3] mmc: tmio: fix reset operation
- [PATCH v2 0/3] mmc: renesas_sdhi: extend quirk selection to handle ES 
revisions
- [PATCH 0/2] renesas: rcar-gen3: add HS400 quirk for SD clock


diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi 
b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index d28ae95405f1152b..6bc041cc63bdc81b 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -762,6 +762,7 @@
vqmmc-supply = <_1p8v>;
bus-width = <8>;
mmc-hs200-1_8v;
+   mmc-hs400-1_8v;
non-removable;
fixed-emmc-driver-type = <1>;
status = "okay";
diff --git a/arch/arm64/boot/dts/renesas/ulcb.dtsi 
b/arch/arm64/boot/dts/renesas/ulcb.dtsi
index 7e6078508ba0ba19..88d03177ae2b327c 100644
--- a/arch/arm64/boot/dts/renesas/ulcb.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb.dtsi
@@ -430,6 +430,7 @@
vqmmc-supply = <_1p8v>;
bus-width = <8>;
mmc-hs200-1_8v;
+   mmc-hs400-1_8v;
non-removable;
status = "okay";
 };
-- 
2.19.1

[PATCH 2/2] clk: renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

From: Niklas Söderlund 

On H3 (ES1.0,ES2.0) and M3-W (ES1.0,ES1.1) the clock setting for HS400
needs a quirk to function properly. The reason for the quirk is that
there are two settings which produces same divider vale for the SDn
clock. On the effected boards the one currently selected results in HS00
not working.

This change uses the same method as the Gen2 CPG driver and simply
ignores the first clock setting as this is the offending one when
selecting the settings. Which of the two possible settings is used have
no effect for SDR104.

Signed-off-by: Niklas Söderlund 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index ff56ac6134111c38..8cc524a29c855dd2 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -227,6 +227,7 @@ struct sd_clock {
unsigned int div_min;
unsigned int div_max;
unsigned int cur_div_idx;
+   bool skip_first;
 };
 
 /* SDn divider
@@ -243,6 +244,10 @@ struct sd_clock {
  *  1 0 2 (4)  0 (2)  8
  *  1 0 3 (8)  0 (2) 16
  *  1 0 4 (16) 0 (2) 32
+ *
+ *  NOTE: There is a quirk option to ignore the first row of the dividers
+ *  table when searching for suitable settings. This is because HS400 on
+ *  early ES versions of H3 and M3-W requires a specific setting to work.
  */
 static const struct sd_div_table cpg_sd_div_table[] = {
 /* CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  sd_div) */
@@ -327,7 +332,7 @@ static int cpg_sd_clock_set_rate(struct clk_hw *hw, 
unsigned long rate,
u32 val;
unsigned int i;
 
-   for (i = 0; i < clock->div_num; i++)
+   for (i = clock->skip_first ? 1 : 0; i < clock->div_num; i++)
if (div == clock->div_table[i].div)
break;
 
@@ -355,7 +360,7 @@ static const struct clk_ops cpg_sd_clock_ops = {
 
 static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
void __iomem *base, const char *parent_name,
-   struct raw_notifier_head *notifiers)
+   struct raw_notifier_head *notifiers, bool skip_first)
 {
struct clk_init_data init;
struct sd_clock *clock;
@@ -377,6 +382,7 @@ static struct clk * __init cpg_sd_clk_register(const struct 
cpg_core_clk *core,
clock->hw.init = 
clock->div_table = cpg_sd_div_table;
clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
+   clock->skip_first = skip_first;
 
sd_fc = readl(clock->csn.reg) & CPG_SD_FC_MASK;
for (i = 0; i < clock->div_num; i++)
@@ -417,19 +423,28 @@ static u32 cpg_quirks __initdata;
 
 #define PLL_ERRATA BIT(0)  /* Missing PLL0/2/4 post-divider */
 #define RCKCR_CKSELBIT(1)  /* Manual RCLK parent selection */
+#define SD_SKIP_FIRST  BIT(2)  /* Skip first clock in SD table */
 
 static const struct soc_device_attribute cpg_quirks_match[] __initconst = {
{
.soc_id = "r8a7795", .revision = "ES1.0",
-   .data = (void *)(PLL_ERRATA | RCKCR_CKSEL),
+   .data = (void *)(PLL_ERRATA | RCKCR_CKSEL | SD_SKIP_FIRST),
},
{
.soc_id = "r8a7795", .revision = "ES1.*",
-   .data = (void *)RCKCR_CKSEL,
+   .data = (void *)(RCKCR_CKSEL | SD_SKIP_FIRST),
+   },
+   {
+   .soc_id = "r8a7795", .revision = "ES2.0",
+   .data = (void *)SD_SKIP_FIRST,
},
{
.soc_id = "r8a7796", .revision = "ES1.0",
-   .data = (void *)RCKCR_CKSEL,
+   .data = (void *)(RCKCR_CKSEL | SD_SKIP_FIRST),
+   },
+   {
+   .soc_id = "r8a7796", .revision = "ES1.1",
+   .data = (void *)SD_SKIP_FIRST,
},
{ /* sentinel */ }
 };
@@ -504,7 +519,8 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct 
device *dev,
 
case CLK_TYPE_GEN3_SD:
return cpg_sd_clk_register(core, base, __clk_get_name(parent),
-  notifiers);
+  notifiers,
+  cpg_quirks & SD_SKIP_FIRST);
 
case CLK_TYPE_GEN3_R:
if (cpg_quirks & RCKCR_CKSEL) {
-- 
2.19.1

[PATCH 0/2] renesas: rcar-gen3: add HS400 quirk for SD clock

[Niklas Söderlund]

From: Niklas Söderlund 

Hi Geert,

This is the result of the SDHI hackathon for a possible solution to the 
clock issue on early ES versions. It is based on the Gen2 solution where 
a row of the possible clock settings are ignored on the effected SoC+ES 
versions. The first row is not effected when reading settings left by 
the bootloader, only when the setting the clock.

This is tested on H3 (ES1.0, ES2.0), M3-W (ES1.0) and M3-N together with 
patches to enable HS400 with great results. No regressions found for 
eMMC HS200/HS400 modes nor for SDR{25,50,104} on any of the SoCs.

Patch 1/2 adds documentation on which settings is used while 2/2 is the 
real change where the quirk is implemented.

Niklas Söderlund (2):
  clk: renesas: rcar-gen3: add documentation for SD clocks
  clk: renesas: rcar-gen3: add HS400 quirk for SD clock

 drivers/clk/renesas/rcar-gen3-cpg.c | 38 -
 1 file changed, 27 insertions(+), 11 deletions(-)

-- 
2.19.1

[PATCH 1/2] clk: renesas: rcar-gen3: add documentation for SD clocks

[Niklas Söderlund]

From: Niklas Söderlund 

Document the known use cases of the different clock settings. This is
useful as different SoC and ES versions uses different settings to do
the same thing as there are more then one combination to achieve the
same SDn clock speed.

Signed-off-by: Niklas Söderlund 
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c 
b/drivers/clk/renesas/rcar-gen3-cpg.c
index 628b63b85d3f09c5..ff56ac6134111c38 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -233,13 +233,13 @@ struct sd_clock {
  * sd_srcfc   sd_fc   div
  * stp_hck   stp_ck(div)  (div) = sd_srcfc x sd_fc
  *---
- *  0 0 0 (1)  1 (4)  4
- *  0 0 1 (2)  1 (4)  8
- *  1 0 2 (4)  1 (4) 16
- *  1 0 3 (8)  1 (4) 32
+ *  0 0 0 (1)  1 (4)  4 : SDR104 / HS200 / HS400 (8 
TAP)
+ *  0 0 1 (2)  1 (4)  8 : SDR50
+ *  1 0 2 (4)  1 (4) 16 : HS / SDR25
+ *  1 0 3 (8)  1 (4) 32 : NS / SDR12
  *  1 0 4 (16) 1 (4) 64
  *  0 0 0 (1)  0 (2)  2
- *  0 0 1 (2)  0 (2)  4
+ *  0 0 1 (2)  0 (2)  4 : SDR104 / HS200 / HS400 (4 
TAP)
  *  1 0 2 (4)  0 (2)  8
  *  1 0 3 (8)  0 (2) 16
  *  1 0 4 (16) 0 (2) 32
-- 
2.19.1

[PATCH v2 3/3] mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x

[Niklas Söderlund]

From: Niklas Söderlund 

The Renesas BSP confirms that H3 ES1.x and M3-W ES1.x do not properly
support HS400. Add a quirk to indicate this and disable HS400 in the MMC
capabilities if the quirk is set.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_core.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 19d89b4dda64c13c..ed2c406fd62b5c7d 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -47,6 +47,7 @@
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
 struct renesas_sdhi_quirks {
+   bool hs400_disabled;
bool hs400_4taps;
 };
 
@@ -607,15 +608,21 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
-static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = {
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w_es1 = {
+   .hs400_disabled = true,
+   .hs400_4taps = true,
+};
+
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_es2 = {
+   .hs400_disabled = false,
.hs400_4taps = true,
 };
 
 static const struct soc_device_attribute sdhi_quirks_match[]  = {
-   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_h3_m3w 
},
-   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_m3w 
},
-   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_h3_m3w 
},
-   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = 
_quirks_h3_m3w_es1 },
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_es2 
},
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = 
_quirks_h3_m3w_es1 },
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = 
_quirks_h3_m3w_es1 },
{ /* Sentinel. */ },
 };
 
@@ -704,6 +711,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_disabled)
+   host->mmc->caps2 &= ~(MMC_CAP2_HS400 | MMC_CAP2_HS400_ES);
+
if (quirks && quirks->hs400_4taps)
mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
 
-- 
2.19.1

[PATCH v2 0/3] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

[Niklas Söderlund]

From: Niklas Söderlund 

Hi,

Recent datasheet updates have made it clear that some quirks are not SoC
specific but SoC + ES version specific. Currently the quirks are
selected using compatibility values but whit this new information that
is not enough.

Patch 1/3 adds support to select quirks based on SoC + ES revision using 
soc_device_match() and converts the only existing quirk. Patch 2/3 
Removes the old method to select quirk based on compatibility string.  
While Patch 3/3 adds a new quirk from the BSP which blacklists some 
known problematic ES versions for HS400. HS400 is not yet enabled 
upstream so blacklisting these ES versions is not a regression of 
functionality.

Based on mmc/next and tested on H3 and M3-N.

Niklas Söderlund (3):
  mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
  mmc: renesas_sdhi: align compatibility properties for H3 and M3-W
  mmc: renesas_sdhi: disable HS400 on H3 ES1.x and M3-W ES1.x

 drivers/mmc/host/renesas_sdhi_core.c  | 36 +++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 20 ++-
 3 files changed, 41 insertions(+), 35 deletions(-)

-- 
2.19.1

[PATCH v2 1/3] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

[Niklas Söderlund]

From: Niklas Söderlund 

Latest datasheet makes it clear that not all ES revisions of the H3 and
M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
unconditionally for these two SoCs. Prepare to handle the quirk based on
SoC revision instead of compatibility value by using soc_device_match()
and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.

The reason for adding a new quirks struct instead of just a flag is that
looking ahead it seems more quirks needs to be handled in a SoC revision
basis.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_core.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index d3ac43c3d0b655dc..19d89b4dda64c13c 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "renesas_sdhi.h"
 #include "tmio_mmc.h"
@@ -45,6 +46,10 @@
 #define SDHI_VER_GEN3_SD   0xcc10
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
+struct renesas_sdhi_quirks {
+   bool hs400_4taps;
+};
+
 static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 {
u32 val;
@@ -602,11 +607,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = {
+   .hs400_4taps = true,
+};
+
+static const struct soc_device_attribute sdhi_quirks_match[]  = {
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_h3_m3w 
},
+   { /* Sentinel. */ },
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
   const struct tmio_mmc_dma_ops *dma_ops)
 {
struct tmio_mmc_data *mmd = pdev->dev.platform_data;
+   const struct renesas_sdhi_quirks *quirks = NULL;
const struct renesas_sdhi_of_data *of_data;
+   const struct soc_device_attribute *attr;
struct tmio_mmc_data *mmc_data;
struct tmio_mmc_dma *dma_priv;
struct tmio_mmc_host *host;
@@ -616,6 +635,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
of_data = of_device_get_match_data(>dev);
 
+   attr = soc_device_match(sdhi_quirks_match);
+   if (attr)
+   quirks = attr->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
@@ -681,6 +704,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_4taps)
+   mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
+
/* For some SoC, we disable internal WP. GPIO may override this */
if (mmc_can_gpio_ro(host->mmc))
mmc_data->capabilities2 &= ~MMC_CAP2_NO_WRITE_PROTECT;
-- 
2.19.1

[PATCH v2 2/3] mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

[Niklas Söderlund]

From: Niklas Söderlund 

It was though all ES revisions of H3 and M3-W SoCs required the
TMIO_MMC_HAVE_4TAP_HS400 flag. Recent datasheet updates tells us this is
not true, only early ES revisions of the SoC do.

Since quirk matching based on ES revisions is now used to handle the
flag it's possible to align all Gen3 compatibility properties. This will
allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400
mode.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 20 +++
 2 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index b6f54102bfdd3a76..68abe042e9f765ad 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -81,22 +81,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-   /* DMAC can handle 0x blk count but only 1 segment */
-   .max_blk_count  = 0x,
-   .max_segs   = 1,
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -114,8 +98,8 @@ static const struct renesas_sdhi_of_data 
of_rcar_gen3_compatible = {
 
 static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-mmc-r8a77470", .data = 
_rcar_gen3_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
{},
 };
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c 
b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 1a4016f635d398c2..8471160316e073c5 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -75,19 +75,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -114,8 +101,8 @@ static const struct of_device_id 
renesas_sdhi_sys_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-r8a7792", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7793", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7794", .data = 
_rcar_gen2_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen1-sdhi", .data = 
_rcar_gen1_compatible, },
{ .compatible = "renesas,rcar-gen2-sdhi", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
@@ -493,8 +480,7 @@ static const struct soc_device_attribute 
gen3_soc

[PATCH] mmc: tmio: delete wait in tuning process

[Niklas Söderlund]

From: Masaharu Hayakawa 

The manual does not contain information that a wait is needed in the
tuning process, this might be a leftover from early development.
Removing the wait don't have any effect on operation so delete the wait
to shorten the initialization time.

Signed-off-by: Masaharu Hayakawa 
Signed-off-by: Takeshi Saito 
[Niklas: fixup commit message]
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 8d64f6196f33e882..a8f917f744fb9f63 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -734,8 +734,6 @@ static int tmio_mmc_execute_tuning(struct mmc_host *mmc, 
u32 opcode)
ret = mmc_send_tuning(mmc, opcode, NULL);
if (ret == 0)
set_bit(i, host->taps);
-
-   usleep_range(1000, 1200);
}
 
ret = host->select_tuning(host);
-- 
2.19.1

[PATCH] mmc: renesas_sdhi: remove workaround for HS400 clock

[Niklas Söderlund]

From: Niklas Söderlund 

The driver sets an incorrect clock and depends on the clock driver
knowledge of this incorrect setting to still set a 200Mhz SDn clock.
Instead of spreading the workaround between the two drivers the clock
driver should be made aware of the ES versions where the special clock
handling is needed no need to keep this workaround in the SDHI driver.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_core.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index d3ac43c3d0b655dc..78bd117bbe65de46 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -163,15 +163,6 @@ static void renesas_sdhi_set_clock(struct tmio_mmc_host 
*host,
if (new_clock == 0)
goto out;
 
-   /*
-* Both HS400 and HS200/SD104 set 200MHz, but some devices need to
-* set 400MHz to distinguish the CPG settings in HS400.
-*/
-   if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400 &&
-   host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400 &&
-   new_clock == 2)
-   new_clock = 4;
-
clock = renesas_sdhi_clk_update(host, new_clock) / 512;
 
for (clk = 0x8080; new_clock >= (clock << 1); clk >>= 1)
-- 
2.19.1

Re: [PATCH] pinctrl: sh-pfc: Reduce kernel size for narrow VIN channels

[Niklas Söderlund]

Hi Geert,

Thanks for your patch.

On 2018-10-16 14:00:36 +0200, Geert Uytterhoeven wrote:
> Some VIN channels support less than 24 lanes.  As union vin_data always
> consumes space for 24 lanes, this wastes memory.
> 
> Hence introduce new smaller unions vin_data12 and vin_data16, to
> accommodate VIN channels with only 12 or 16 lanes.
> 
> This reduces the static pin controller driver size by 320 bytes for
> R-Car V2H, and by 96 bytes for R-Car E2.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Niklas Söderlund 

> ---
> To be queued in sh-pfc-for-v4.21.
> 
>  drivers/pinctrl/sh-pfc/pfc-r8a7792.c | 16 
>  drivers/pinctrl/sh-pfc/pfc-r8a7794.c |  4 ++--
>  drivers/pinctrl/sh-pfc/sh_pfc.h  | 17 +++--
>  3 files changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7792.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
> index bf0681b381819a4b..e977121b433b773c 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7792.c
> @@ -1474,7 +1474,7 @@ static const unsigned int vin1_clk_mux[] = {
>   VI1_CLK_MARK,
>  };
>  /* - VIN2 
> --- */
> -static const union vin_data vin2_data_pins = {
> +static const union vin_data16 vin2_data_pins = {
>   .data16 = {
>   RCAR_GP_PIN(6, 4), RCAR_GP_PIN(6, 5),
>   RCAR_GP_PIN(6, 6), RCAR_GP_PIN(6, 7),
> @@ -1486,7 +1486,7 @@ static const union vin_data vin2_data_pins = {
>   RCAR_GP_PIN(8, 11), RCAR_GP_PIN(8, 12),
>   },
>  };
> -static const union vin_data vin2_data_mux = {
> +static const union vin_data16 vin2_data_mux = {
>   .data16 = {
>   VI2_D0_C0_MARK, VI2_D1_C1_MARK,
>   VI2_D2_C2_MARK, VI2_D3_C3_MARK,
> @@ -1524,7 +1524,7 @@ static const unsigned int vin2_clk_mux[] = {
>   VI2_CLK_MARK,
>  };
>  /* - VIN3 
> --- */
> -static const union vin_data vin3_data_pins = {
> +static const union vin_data16 vin3_data_pins = {
>   .data16 = {
>   RCAR_GP_PIN(7, 4), RCAR_GP_PIN(7, 5),
>   RCAR_GP_PIN(7, 6), RCAR_GP_PIN(7, 7),
> @@ -1536,7 +1536,7 @@ static const union vin_data vin3_data_pins = {
>   RCAR_GP_PIN(8, 15), RCAR_GP_PIN(8, 16),
>   },
>  };
> -static const union vin_data vin3_data_mux = {
> +static const union vin_data16 vin3_data_mux = {
>   .data16 = {
>   VI3_D0_C0_MARK, VI3_D1_C1_MARK,
>   VI3_D2_C2_MARK, VI3_D3_C3_MARK,
> @@ -1574,7 +1574,7 @@ static const unsigned int vin3_clk_mux[] = {
>   VI3_CLK_MARK,
>  };
>  /* - VIN4 
> --- */
> -static const union vin_data vin4_data_pins = {
> +static const union vin_data12 vin4_data_pins = {
>   .data12 = {
>   RCAR_GP_PIN(8, 4), RCAR_GP_PIN(8, 5),
>   RCAR_GP_PIN(8, 6), RCAR_GP_PIN(8, 7),
> @@ -1584,7 +1584,7 @@ static const union vin_data vin4_data_pins = {
>   RCAR_GP_PIN(8, 14), RCAR_GP_PIN(8, 15),
>   },
>  };
> -static const union vin_data vin4_data_mux = {
> +static const union vin_data12 vin4_data_mux = {
>   .data12 = {
>   VI4_D0_C0_MARK, VI4_D1_C1_MARK,
>   VI4_D2_C2_MARK, VI4_D3_C3_MARK,
> @@ -1620,7 +1620,7 @@ static const unsigned int vin4_clk_mux[] = {
>   VI4_CLK_MARK,
>  };
>  /* - VIN5 
> --- */
> -static const union vin_data vin5_data_pins = {
> +static const union vin_data12 vin5_data_pins = {
>   .data12 = {
>   RCAR_GP_PIN(9, 4), RCAR_GP_PIN(9, 5),
>   RCAR_GP_PIN(9, 6), RCAR_GP_PIN(9, 7),
> @@ -1630,7 +1630,7 @@ static const union vin_data vin5_data_pins = {
>   RCAR_GP_PIN(9, 14), RCAR_GP_PIN(9, 15),
>   },
>  };
> -static const union vin_data vin5_data_mux = {
> +static const union vin_data12 vin5_data_mux = {
>   .data12 = {
>   VI5_D0_C0_MARK, VI5_D1_C1_MARK,
>   VI5_D2_C2_MARK, VI5_D3_C3_MARK,
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c 
> b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> index 6d1e5fdc03f84554..b96a3cc79084ddcc 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7794.c
> @@ -3704,7 +3704,7 @@ static const unsigned int vin0_clk_mux[] = {
>   VI0_CLK_MARK,
>  };
>  /* - VIN1 
> --- */
> -static const union vin_data vin1_data_pins = {
> +static const union vin_data12 vin1_data_pins = {
>   

[PATCH v2 2/3] mmc: tmio: fix reset operation

[Niklas Söderlund]

From: Niklas Söderlund 

SD / MMC did not operate properly when suspend transition failed.
Because the SCC was not reset at resume, issue of the command failed.
Merge tmio_mmc_reset() into tmio_mmc_hw_reset() in order to add reset
of SCC to tmio_mmc_host_runtime_resume().

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 

---
* Changes sine v1
- Merge tmio_mmc_reset() into tmio_mmc_hw_reset() as it's now the only
  caller.
---
 drivers/mmc/host/tmio_mmc_core.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 71d3b380760d425f..da7a5cf0b7ae 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -157,8 +157,10 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, 
int enable)
}
 }
 
-static void tmio_mmc_reset(struct tmio_mmc_host *host)
+static void tmio_mmc_hw_reset(struct mmc_host *mmc)
 {
+   struct tmio_mmc_host *host = mmc_priv(mmc);
+
/* FIXME - should we set stop clock reg here */
sd_ctrl_write16(host, CTL_RESET_SD, 0x);
if (host->pdata->flags & TMIO_MMC_HAVE_HIGH_REG)
@@ -174,6 +176,10 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
}
 
+   tmio_mmc_abort_dma(host);
+
+   if (host->hw_reset)
+   host->hw_reset(host);
 }
 
 static void tmio_mmc_reset_work(struct work_struct *work)
@@ -214,7 +220,7 @@ static void tmio_mmc_reset_work(struct work_struct *work)
 
spin_unlock_irqrestore(>lock, flags);
 
-   tmio_mmc_reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
/* Ready for new calls */
host->mrq = NULL;
@@ -701,14 +707,6 @@ static int tmio_mmc_start_data(struct tmio_mmc_host *host,
return 0;
 }
 
-static void tmio_mmc_hw_reset(struct mmc_host *mmc)
-{
-   struct tmio_mmc_host *host = mmc_priv(mmc);
-
-   if (host->hw_reset)
-   host->hw_reset(host);
-}
-
 static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 {
struct tmio_mmc_host *host = mmc_priv(mmc);
@@ -1230,7 +1228,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 
_host->set_clock(_host, 0);
-   tmio_mmc_reset(_host);
+   tmio_mmc_hw_reset(mmc);
 
_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
CTL_IRQ_MASK);
tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
@@ -1331,7 +1329,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
tmio_mmc_clk_enable(host);
-   tmio_mmc_reset(host);
+   tmio_mmc_hw_reset(host->mmc);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

[PATCH v2 3/3] mmc: renesas_sdhi: add initial setting of interrupt mask register

[Niklas Söderlund]

From: Niklas Söderlund 

The initial value of the interrupt mask register may be different from
the H/W manual at the startup of the kernel by setting from the
bootloader. Since the error interrupts may be unmasked, the driver sets
initial value.

The initial value is only known for R-Car Gen2 and Gen3 platforms so
limit the initialization to those platforms.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 

---

* Changes since v1
- Limit the initialization to Gen2+ platforms by checking the
  TMIO_MMC_MIN_RCAR2 flag.
- Rename the constant for the initialization value to reflect it's only
  for R-Car Gen2+ platforms.
- Move setting of the initial mask to renesas_sdhi.
---
 drivers/mmc/host/renesas_sdhi_core.c | 4 
 drivers/mmc/host/tmio_mmc.h  | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index d3ac43c3d0b655dc..f2162f2b7de3ae05 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -532,6 +532,10 @@ static void renesas_sdhi_hw_reset(struct tmio_mmc_host 
*host)
sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL,
   ~SH_MOBILE_SDHI_SCC_RVSCNTL_RVSEN &
   sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL));
+
+   if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
+   sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK,
+TMIO_MASK_INIT_RCAR2);
 }
 
 static int renesas_sdhi_wait_idle(struct tmio_mmc_host *host, u32 bit)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a9972dc60c6fbb8c..00673cec47a4de13 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -99,6 +99,7 @@
 
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
+#define TMIO_MASK_INIT_RCAR2   0x8b7f031d /* Initial value for R-Car Gen2+ */
 #define TMIO_MASK_ALL   0x837f031d
 #define TMIO_MASK_READOP  (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND)
 #define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND)
-- 
2.19.1

[PATCH v2 1/3] mmc: tmio: enable module clock before resetting when resuming

[Niklas Söderlund]

From: Niklas Söderlund 

On runtime power management resume, the host clock needs to be
enabled before calling tmio_mmc_reset. If the mmc device has a power
domain entry, the host clock is enabled via genpd_runtime_resume,
running before tmio_mmc_host_runtime_resume. If the mmc device has no
power domain entry, however, genpd_runtime_resume is not called. This
patch changes tmio_mmc_host_runtime_resume to enable the host clock
before calling tmio_mmc_reset.

Based on work from Masaharu Hayakawa.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index f05c3a622f090cd6..71d3b380760d425f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1330,8 +1330,8 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 {
struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
-   tmio_mmc_reset(host);
tmio_mmc_clk_enable(host);
+   tmio_mmc_reset(host);
 
if (host->clk_cache)
host->set_clock(host, host->clk_cache);
-- 
2.19.1

[PATCH v2 0/3] mmc: tmio: Fix reset operation

[Niklas Söderlund]

From: Niklas Söderlund 

Hi,

While looking at the Renesas BSP kernel I found patches which improves
the state of the hardware at probe and after runtime resume.

Patch 1/3 make sure the module clock is enabled after resuming before 
register are accessed. Patch 2/3 is the real change in this series and 
brings in reset of the vendor specific callback when resetting (SCC in 
the Renesas case). While 3/3 simply make sure that the IRQ mask for 
Renesas boards (Gen2 and later) are in a good shape after probe (and 
reset).

In addition to addressing the state after resuming it helped unbreak a 
SD card I have which are very problematic on Koelsch. Without this 
series inserting the card results in:

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
sh_mobile_sdhi ee10.sd: Tuning procedure failed
mmc0: tuning execution failed: -5
mmc0: error -5 whilst initialising SD card
sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)

While with this series applied (patch 2/3):

sh_mobile_sdhi ee10.sd: timeout waiting for hardware interrupt (CMD19)
mmc0: new ultra high speed SDR50 SDHC card at address 
mmcblk0: mmc0: SU04G 3.69 GiB 
 mmcblk0: p1 p2

Patch 1/3 was previously part of 2/3 but as it deals with a unrelated 
issue it's here broken out to a separate patch. Patch 3/3 have once been 
posted outside this series bit after comments from Wolfram it's brought 
back as it now deepens on 2/3.

Most changes in this series are based on similar work from Masaharu 
Hayakawa but for this version all changes now differ quiet a lot from 
his work.  All mails sent to him bounce with a "Undelivered Mail 
Returned to Sender" error. I therefor felt the need to claim authorship 
as I don't want to post blame of my (potential) mistakes on some else.

Niklas Söderlund (3):
  mmc: tmio: enable module clock before resetting when resuming
  mmc: tmio: fix reset operation
  mmc: renesas_sdhi: add initial setting of interrupt mask register

 drivers/mmc/host/renesas_sdhi_core.c |  4 
 drivers/mmc/host/tmio_mmc.h  |  1 +
 drivers/mmc/host/tmio_mmc_core.c | 22 ++
 3 files changed, 15 insertions(+), 12 deletions(-)

-- 
2.19.1

Re: [PATCH v2] arm64: dts: renesas: r8a77980: add thermal support

[Niklas Söderlund]

Hi Geert,

On 2018-10-11 09:02:22 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Thu, Oct 11, 2018 at 12:11 AM Niklas Söderlund
>  wrote:
> > On 2018-10-10 22:18:11 +0300, Sergei Shtylyov wrote:
> > > Describe THS/CIVM in the R8A77980 device trees.
> > >
> > > Signed-off-by: Sergei Shtylyov 
> 
> > > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > > @@ -330,6 +330,19 @@
> > >   #power-domain-cells = <1>;
> > >   };
> > >
> > > + tsc: thermal@e6198000 {
> > > + compatible = "renesas,r8a77980-thermal";
> > > + reg = <0 0xe6198000 0 0x100>,
> > > +   <0 0xe61a 0 0x100>;
> > > + interrupts = ,
> > > +  ,
> > > +  ;
> > > + clocks = < CPG_MOD 522>;
> > > + power-domains = < R8A77980_PD_ALWAYS_ON>;
> > > + resets = < 522>;
> > > + #thermal-sensor-cells = <1>;
> >
> > The status property is missing but as you told me in v1 it should not
> > matter. I will leave it for Simon to decide if he wants it to keep it
> > consistent with other SoC or if we should remove it from the other dtsi
> > files. In any case with or without the status property.
> 
> Forgot to review commit c79661eb5060e2bf ("arm64: dts: renesas: Remove
> unneeded status from thermal nodes")? ;-)

Not only that also reviewing using the context from v4.19-rc1 which of 
course is not correct for dtsi patches, thanks for enlightening me :-)

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] arm64: dts: renesas: r8a77980: add thermal support

[Niklas Söderlund]

Hi Sergei,

Thanks for keep working on this patch.

On 2018-10-10 22:18:11 +0300, Sergei Shtylyov wrote:
> Describe THS/CIVM in the R8A77980 device trees.
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'renesas-devel-20181008-v4.19-rc7' tag of Simon
> Horman's 'renesas.git' repo.
> 
> Changes in version 2:
> - renamed the thermal device node label;
> - renamed the thermal zone nodes;
> - added the passive trip point in the 1st thermal zone and the passive and
>   critical trip points in the 2nd thermal zone;
> - changed the "hysteresis" prop in the critical trip point;
> - removed the empty "cooling-maps" node from the 1st thermal zone.
> 
>  arch/arm64/boot/dts/renesas/r8a77980.dtsi |   53 
> ++
>  1 file changed, 53 insertions(+)
> 
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -330,6 +330,19 @@
>   #power-domain-cells = <1>;
>   };
>  
> + tsc: thermal@e6198000 {
> + compatible = "renesas,r8a77980-thermal";
> + reg = <0 0xe6198000 0 0x100>,
> +   <0 0xe61a 0 0x100>;
> + interrupts = ,
> +  ,
> +  ;
> + clocks = < CPG_MOD 522>;
> + power-domains = < R8A77980_PD_ALWAYS_ON>;
> + resets = < 522>;
> + #thermal-sensor-cells = <1>;

The status property is missing but as you told me in v1 it should not 
matter. I will leave it for Simon to decide if he wants it to keep it 
consistent with other SoC or if we should remove it from the other dtsi 
files. In any case with or without the status property.

Reviewed-by: Niklas Söderlund 

> + };
> +
>   intc_ex: interrupt-controller@e61c {
>   compatible = "renesas,intc-ex-r8a77980", "renesas,irqc";
>   #interrupt-cells = <2>;
> @@ -1404,6 +1417,46 @@
>   };
>   };
>  
> + thermal-zones {
> + thermal-sensor-1 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = < 0>;
> +
> + trips {
> + sensor1-passive {
> + temperature = <95000>;
> + hysteresis = <1000>;
> + type = "passive";
> + };
> + sensor1-critical {
> + temperature = <12>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + thermal-sensor-2 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = < 1>;
> +
> + trips {
> + sensor2-passive {
> + temperature = <95000>;
> + hysteresis = <1000>;
> + type = "passive";
> + };
> + sensor2-critical {
> + temperature = <12>;
> + hysteresis = <1000>;
> + type = "critical";
> + };
> + };
> + };
> + };
> +
>   timer {
>   compatible = "arm,armv8-timer";
>   interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register

[Niklas Söderlund]

Hi Wolfram,

Thanks for our feedback.

On 2018-10-10 01:53:58 +0200, Wolfram Sang wrote:
> Hi Niklas,
> 
> > There are already checks for TMIO_MMC_MIN_RCAR2 inside 
> > tmio_mmc_host_probe(), but I agree with you it would be good if instead 
> > of adding to that start to move Renesas specific code out.
> 
> Thanks!
> 
> > I did a quick test and it seems sane to move this to the end of 
> > renesas_sdhi_hw_reset(). Before I send a v3 of this what is your view?
> 
> It seems a good place to me if it also gets called when probing the
> device. From a quick glimpse, I see that this function ends up in
> mmc_ops->hw_reset, but I haven't verified that the MMC core calls it
> during probe. But you said you tested it...
> 

You are correct, in mmc/next it is not called during probe. I see now I 
did my testing on-top of the reset series I'm currently reworking and 
then it's called at the right time to use the initial set irq mask for 
the _host->sdcard_irq_mask read out in tmio_mmc_host_probe().

I will move this patch back to the top of the reset series as it would 
IMHO create the least amount of code churn. Another option is to merge 
this version now and move it once the reset series is accepted.

-- 
Regards,
Niklas Söderlund

Re: [PATCH] arm64: dts: renesas: r8a77980: add thermal support

[Niklas Söderlund]

Hi Sergei,

Thanks for your work.

On 2018-10-09 22:37:47 +0300, Sergei Shtylyov wrote:
> Describe THS/CIVM in the R8A77980 device trees.
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch is against the 'renesas-devel-20181008-v4.19-rc7' tag of Simon
> Horman's 'renesas.git' repo.
> 
> The thermal driver/bindings patches have been just posted...
> 
>  arch/arm64/boot/dts/renesas/r8a77980.dtsi |   38 
> ++
>  1 file changed, 38 insertions(+)
> 
> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> ===
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -330,6 +330,19 @@
>   #power-domain-cells = <1>;
>   };
>  
> + thermal: thermal@e6198000 {

As Simon points out other Gen3 thermal nodes use "tsc:" not "thermal:".

> + compatible = "renesas,r8a77980-thermal";
> + reg = <0 0xe6198000 0 0x100>,
> +   <0 0xe61a 0 0x100>;
> + interrupts = ,
> +  ,
> +  ;
> + clocks = < CPG_MOD 522>;
> + power-domains = < R8A77980_PD_ALWAYS_ON>;
> + resets = < 522>;
> + #thermal-sensor-cells = <1>;

Missing status = "okay" or am I missing something?

> + };
> +
>   intc_ex: interrupt-controller@e61c {
>   compatible = "renesas,intc-ex-r8a77980", "renesas,irqc";
>   #interrupt-cells = <2>;
> @@ -1404,6 +1417,31 @@
>   };
>   };
>  
> + thermal-zones {
> + cpu-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = < 0>;
> +
> + trips {
> + cpu-crit {
> + temperature = <12>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + };
> + };
> +
> + sensor2-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> + thermal-sensors = < 1>;
> + };
> + };

The thermal-zones node uses the Gen2 labels and I think this should be 
updated to be as consistent as possible with other Gen3 users. For extra 
points expand this could be expanded to also include the cooling-maps 
but could also happen in a separate patch if cooling-devices are not yet 
defined :-)

> +
>   timer {
>   compatible = "arm,armv8-timer";
>   interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |

-- 
Regards,
Niklas Söderlund

Re: [PATCH] arm64: dts: renesas: r8a77980: add thermal support

[Niklas Söderlund]

Hi Sergei,

On 2018-10-10 13:47:36 +0300, Sergei Shtylyov wrote:
> On 10/10/2018 11:36 AM, Simon Horman wrote:
> 
> >> Describe THS/CIVM in the R8A77980 device trees.
> >>
> >> Signed-off-by: Sergei Shtylyov 
> >>
> >> ---
> >> This patch is against the 'renesas-devel-20181008-v4.19-rc7' tag of Simon
> >> Horman's 'renesas.git' repo.
> >>
> >> The thermal driver/bindings patches have been just posted...
> >>
> >>  arch/arm64/boot/dts/renesas/r8a77980.dtsi |   38 
> >> ++
> > 
> > Thanks Sergei, one minor nit from me below.
> > 
> > Niklas, I'd value your review of this patch.
> > 
> >>  1 file changed, 38 insertions(+)
> >>
> >> Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> >> ===
> >> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> >> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> >> @@ -330,6 +330,19 @@
> >>#power-domain-cells = <1>;
> >>};
> >>  
> >> +  thermal: thermal@e6198000 {
> > 
> > "tsc:" would be consistent with other R-Car Gen 3 dtsi that
> > describe this device.
> 
>There's no consistency even among those: R8A779{5|6}0 use "tsc:", 
> R8A77995 uses "thermal:"... :-)

D3 (R8A77995) uses the Gen2 thermal driver while V3H uses Gen3 thermal 
driver, so there is some consistency and I think we should aim to make 
it as consistent as possible :-)

> 
> [...]
> 
> MBR, Sergei

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] thermal: rcar_gen3_thermal: add R8A77980 support

[Niklas Söderlund]

Hi Sergei,

Thanks for your patch.

On 2018-10-09 22:11:51 +0300, Sergei Shtylyov wrote:
> Add the R-Car V3H (R8A77980) SoC support to the R-Car gen3 thermal driver.
> 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Niklas Söderlund 

> 
> ---
>  drivers/thermal/rcar_gen3_thermal.c |1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-soc-thermal/drivers/thermal/rcar_gen3_thermal.c
> ===
> --- linux-soc-thermal.orig/drivers/thermal/rcar_gen3_thermal.c
> +++ linux-soc-thermal/drivers/thermal/rcar_gen3_thermal.c
> @@ -321,6 +321,7 @@ static const struct of_device_id rcar_ge
>   { .compatible = "renesas,r8a7795-thermal", },
>   { .compatible = "renesas,r8a7796-thermal", },
>   { .compatible = "renesas,r8a77965-thermal", },
> + { .compatible = "renesas,r8a77980-thermal", },
>   {},
>  };
>  MODULE_DEVICE_TABLE(of, rcar_gen3_thermal_dt_ids);

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] dt-bindings: thermal: rcar-gen3-thermal: document R8A77980 bindings

[Niklas Söderlund]

Hi Sergei,

Thanks for your work.

On 2018-10-09 22:10:14 +0300, Sergei Shtylyov wrote:
> Document the R-Car V3H (R8A77980) SoC in the Renesas R-Car gen3 thermal
> bindings.
> 
> Signed-off-by: Sergei Shtylyov 

Reviewed-by: Niklas Söderlund 

> 
> ---
>  Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Index: 
> linux-soc-thermal/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
> ===
> --- 
> linux-soc-thermal.orig/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
> +++ 
> linux-soc-thermal/Documentation/devicetree/bindings/thermal/rcar-gen3-thermal.txt
> @@ -10,6 +10,7 @@ Required properties:
>   - "renesas,r8a7795-thermal" (R-Car H3)
>   - "renesas,r8a7796-thermal" (R-Car M3-W)
>   - "renesas,r8a77965-thermal" (R-Car M3-N)
> + - "renesas,r8a77980-thermal" (R-Car V3H)
>  - reg: Address ranges of the thermal registers. Each 
> sensor
> needs one address range. Sorting must be done in
> increasing order according to datasheet, i.e.
> @@ -19,7 +20,8 @@ Required properties:
>  
>  Optional properties:
>  
> -- interrupts   : interrupts routed to the TSC (3 for H3, M3-W and 
> M3-N)
> +- interrupts : interrupts routed to the TSC (3 for H3, M3-W, M3-N,
> +   and V3H)
>  - power-domain   : Must contain a reference to the power domain. 
> This
>     property is mandatory if the thermal sensor instance
> is part of a controllable power domain.

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register

[Niklas Söderlund]

Hi Wolfram,

Thanks for your feedback.

On 2018-10-03 00:54:02 +0200, Wolfram Sang wrote:
> On Wed, Sep 26, 2018 at 05:00:26PM +0200, Niklas Söderlund wrote:
> > From: Masaharu Hayakawa 
> > 
> > The initial value of the interrupt mask register may be different from
> > the H/W manual at the startup of the kernel by setting from the
> > bootloader. Since the error interrupts may be unmasked, the driver sets
> > initial value.
> > 
> > The initial value is only known for R-Car Gen2 and Gen3 platforms so
> > limit the initialization to those platforms.
> > 
> > Signed-off-by: Masaharu Hayakawa 
> > Signed-off-by: Niklas Söderlund 
> > 
> > ---
> > 
> > * Changes since v1
> > - Limit the initialization to Gen2+ platforms by checking the
> >   TMIO_MMC_MIN_RCAR2 flag.
> > - Rename the constant for the initialization value to reflect it's only
> >   for R-Car Gen2+ platforms.
> 
> Those changes are good! I wonder, though, if we shouldn't move the code
> out of TMIO core into the SDHI core? This seems very Renesas specific.
> What do you think?
> 

There are already checks for TMIO_MMC_MIN_RCAR2 inside 
tmio_mmc_host_probe(), but I agree with you it would be good if instead 
of adding to that start to move Renesas specific code out.

I did a quick test and it seems sane to move this to the end of 
renesas_sdhi_hw_reset(). Before I send a v3 of this what is your view?

-- 
Regards,
Niklas Söderlund

[PATCH v2] mmc: tmio: Add initial setting of interrupt mask register

[Niklas Söderlund]

From: Masaharu Hayakawa 

The initial value of the interrupt mask register may be different from
the H/W manual at the startup of the kernel by setting from the
bootloader. Since the error interrupts may be unmasked, the driver sets
initial value.

The initial value is only known for R-Car Gen2 and Gen3 platforms so
limit the initialization to those platforms.

Signed-off-by: Masaharu Hayakawa 
Signed-off-by: Niklas Söderlund 

---

* Changes since v1
- Limit the initialization to Gen2+ platforms by checking the
  TMIO_MMC_MIN_RCAR2 flag.
- Rename the constant for the initialization value to reflect it's only
  for R-Car Gen2+ platforms.
---
 drivers/mmc/host/tmio_mmc.h  | 1 +
 drivers/mmc/host/tmio_mmc_core.c | 4 
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a9972dc60c6fbb8c..00673cec47a4de13 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -99,6 +99,7 @@
 
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
+#define TMIO_MASK_INIT_RCAR2   0x8b7f031d /* Initial value for R-Car Gen2+ */
 #define TMIO_MASK_ALL   0x837f031d
 #define TMIO_MASK_READOP  (TMIO_STAT_RXRDY | TMIO_STAT_DATAEND)
 #define TMIO_MASK_WRITEOP (TMIO_STAT_TXRQ | TMIO_STAT_DATAEND)
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index f05c3a622f090cd6..5aae7e2129400671 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1232,6 +1232,10 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
_host->set_clock(_host, 0);
tmio_mmc_reset(_host);
 
+   if (pdata->flags & TMIO_MMC_MIN_RCAR2)
+   sd_ctrl_write32_as_16_and_16(_host, CTL_IRQ_MASK,
+TMIO_MASK_INIT_RCAR2);
+
_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, 
CTL_IRQ_MASK);
tmio_mmc_disable_mmc_irqs(_host, TMIO_MASK_ALL);
 
-- 
2.19.0

Re: [PATCH 1/3] mmc: tmio: Add initial setting of interrupt mask register

[Niklas Söderlund]

Hi Wolfram,

On 2018-09-16 16:48:07 +0200, Wolfram Sang wrote:
> 
> > > > > So, we should at least protect this with TMIO_MMC_MIN_RCAR2, I'd 
> > > > > think.
> > > > > Maybe we should even move it to renesas_sdhi_core.c, but I am not too
> > > > > sure about that.
> > > 
> > > ... this.
> > > 
> > 
> > I will look into this and send a new version :-)
> 
> Did you send V2 of this? I can't find it.
> 

No I have not yet posted a v2 of this series. Perhaps I should break 
this patch out and send separate? I have handled the comments for this 
but patch 3/3 in this series still requires more work, what do you 
think?

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 4/8] media: rcar-csi2: Add R8A77990 support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:41 +0200, Jacopo Mondi wrote:
> Add support for R-Car E3 R8A77965 to R-Car CSI-2 driver.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi 

Acked-by: Niklas Söderlund 

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c 
> b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index dc5ae80..f82b668 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -959,6 +959,11 @@ static const struct rcar_csi2_info 
> rcar_csi2_info_r8a77970 = {
>   .confirm_start = rcsi2_confirm_start_v3m_e3,
>  };
>  
> +static const struct rcar_csi2_info rcar_csi2_info_r8a77990 = {
> + .init_phtw = rcsi2_init_phtw_v3m_e3,
> + .confirm_start = rcsi2_confirm_start_v3m_e3,
> +};
> +
>  static const struct of_device_id rcar_csi2_of_table[] = {
>   {
>   .compatible = "renesas,r8a7795-csi2",
> @@ -976,6 +981,10 @@ static const struct of_device_id rcar_csi2_of_table[] = {
>   .compatible = "renesas,r8a77970-csi2",
>   .data = _csi2_info_r8a77970,
>   },
> + {
> + .compatible = "renesas,r8a77990-csi2",
> + .data = _csi2_info_r8a77990,
> + },
>   { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 3/8] media: dt-bindings: rcar-csi2: Add R8A77990

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:40 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of supported SoCs.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Rob Herring 

Acked-by: Niklas Söderlund 

> ---
>  Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt 
> b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> index 2d385b6..2824489 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -12,6 +12,7 @@ Mandatory properties
> - "renesas,r8a7796-csi2" for the R8A7796 device.
> - "renesas,r8a77965-csi2" for the R8A77965 device.
> - "renesas,r8a77970-csi2" for the R8A77970 device.
> +   - "renesas,r8a77990-csi2" for the R8A77990 device.
>  
>   - reg: the register base and size for the device registers
>   - interrupts: the interrupt for the device
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 2/8] media: rcar-vin: Add support for R-Car R8A77990

[Niklas Söderlund]

Hi Jacopo,

Thanks for for work.

On 2018-09-05 17:29:39 +0200, Jacopo Mondi wrote:
> Add R-Car E3 R8A77990 SoC to the rcar-vin supported ones.
> Based on the experimental patch from Magnus Damm.
> 
> Signed-off-by: Jacopo Mondi 

Acked-by: Niklas Söderlund 

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index ce09799..d443c09 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -1085,6 +1085,22 @@ static const struct rvin_info rcar_info_r8a77970 = {
>   .routes = rcar_info_r8a77970_routes,
>  };
>  
> +static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
> + { .csi = RVIN_CSI40, .channel = 0, .vin = 4, .mask = BIT(0) | BIT(3) },
> + { .csi = RVIN_CSI40, .channel = 0, .vin = 5, .mask = BIT(2) },
> + { .csi = RVIN_CSI40, .channel = 1, .vin = 4, .mask = BIT(2) },
> + { .csi = RVIN_CSI40, .channel = 1, .vin = 5, .mask = BIT(1) | BIT(3) },
> + { /* Sentinel */ }
> +};
> +
> +static const struct rvin_info rcar_info_r8a77990 = {
> + .model = RCAR_GEN3,
> + .use_mc = true,
> + .max_width = 4096,
> + .max_height = 4096,
> + .routes = rcar_info_r8a77990_routes,
> +};
> +
>  static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
>   { /* Sentinel */ }
>  };
> @@ -1143,6 +1159,10 @@ static const struct of_device_id rvin_of_id_table[] = {
>   .data = _info_r8a77970,
>   },
>   {
> + .compatible = "renesas,vin-r8a77990",
> + .data = _info_r8a77990,
> + },
> + {
>   .compatible = "renesas,vin-r8a77995",
>   .data = _info_r8a77995,
>   },
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 1/8] media: dt-bindings: rcar-vin: Add R8A77990 support

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-09-05 17:29:38 +0200, Jacopo Mondi wrote:
> Add compatible string for R-Car E3 R8A77990 to the list of SoCs supported by
> rcar-vin driver.
> 
> Signed-off-by: Jacopo Mondi 
> Reviewed-by: Rob Herring 

Acked-by: Niklas Söderlund 

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt 
> b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 2f42005..dfd6058 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -23,6 +23,7 @@ on Gen3 platforms to a CSI-2 receiver.
> - "renesas,vin-r8a7796" for the R8A7796 device
> - "renesas,vin-r8a77965" for the R8A77965 device
> - "renesas,vin-r8a77970" for the R8A77970 device
> +   - "renesas,vin-r8a77990" for the R8A77990 device
> - "renesas,vin-r8a77995" for the R8A77995 device
> - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
>   device.
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

Hi Wolfram,

On 2018-08-31 13:04:59 +0200, Wolfram Sang wrote:
> On Fri, Aug 31, 2018 at 12:49:08PM +0200, Niklas Söderlund wrote:
> > On 2018-08-31 12:35:24 +0200, Wolfram Sang wrote:
> > > 
> > > > Promote 
> > > > drivers/gpu/drm/exynos/exynos_drm_iommu.c:configure_dma_max_seg_size()
> > > > to a generic helper?
> > > 
> > > Yes!
> > > 
> > If that is promoted should not
> > drivers/gpu/drm/exynos/exynos_drm_iommu.c:clear_dma_max_seg_size() also 
> > be promoted? And if so should this patch revert back to v1 with a custom 
> > remove function which clears and free the dma_parms ?
> 
> My preference would be easy to use helpers for drivers because this is
> easy to get wrong. I think we need to discuss with the creators of that
> API:
> 
> a) if the drm/exynos driver does the right thing(tm)
> b) if these functions should be generic helpers
> c) when to clear the pointer (a bit related to a))
> 
> Niklas, do you have an interest to do that? Or would you rather go with
> SDHI hacking? :) I can do it, too.
> 

If you would like to spearhead this please go a head :-) If not please 
let me know so it won't fall thru the cracks.

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

On 2018-08-31 12:35:24 +0200, Wolfram Sang wrote:
> 
> > Promote 
> > drivers/gpu/drm/exynos/exynos_drm_iommu.c:configure_dma_max_seg_size()
> > to a generic helper?
> 
> Yes!
> 

If that is promoted should not
drivers/gpu/drm/exynos/exynos_drm_iommu.c:clear_dma_max_seg_size() also 
be promoted? And if so should this patch revert back to v1 with a custom 
remove function which clears and free the dma_parms ?

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2018-08-31 00:10:28 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> Thanks for your patch!
> 
> On Thu, Aug 30, 2018 at 11:39 PM Niklas Söderlund
>  wrote:
> > Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
> > device_dma_parameters structure and filling in the max segment size. The
> > size used is the result of a discussion with Renesas hardware engineers
> > and unfortunately not found in the datasheet.
> >
> >   renesas_sdhi_internal_dmac ee14.sd: DMA-API: mapping sg segment
> >   longer than device claims to support [len=126976] [max=65536A]
> 
> Bogus trailing "A".

Thanks, that is a odd copy-and-past error :-)

> 
> > Signed-off-by: Niklas Söderlund 
> > Reported-by: Geert Uytterhoeven 
> >
> > ---
> >
> > * Changes since v1
> > - Use devm_kzalloc() instead of a custom remove function to clean up.
> > ---
> >  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
> > b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > index c9dab9ce1ed55174..715e0c5dc30e4ebf 100644
> > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> > @@ -308,12 +308,23 @@ static const struct soc_device_attribute 
> > gen3_soc_whitelist[] = {
> >  static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
> >  {
> > const struct soc_device_attribute *soc = 
> > soc_device_match(gen3_soc_whitelist);
> > +   struct device *dev = >dev;
> >
> > if (!soc)
> > return -ENODEV;
> >
> > global_flags |= (unsigned long)soc->data;
> >
> > +   if (!dev->dma_parms) {
> 
> I guess dev->dma_parms is retained on unbind/rebind?
> 
> > +   dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
> > + GFP_KERNEL);
> 
> But by using devm_kzalloc(), the memory will be freed, and lead to reuse after
> free?

I don't know how the unbind/rebind behaves in this regard. In v1 of this 
patch I used kzalloc() and a remove function to free the memory and 
reset the dev->dma_parms pointe to NULL. Do you think that is the 
correct approach here?

> 
> > +   if (!dev->dma_parms)
> > +   return -ENOMEM;
> > +   }
> > +
> > +   if (dma_set_max_seg_size(dev, 0x))
> > +   dev_warn(dev, "Unable to set seg size\n");
> > +
> > return renesas_sdhi_probe(pdev, 
> > _sdhi_internal_dmac_dma_ops);
> >  }
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

[PATCH v2] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
device_dma_parameters structure and filling in the max segment size. The
size used is the result of a discussion with Renesas hardware engineers
and unfortunately not found in the datasheet.

  renesas_sdhi_internal_dmac ee14.sd: DMA-API: mapping sg segment
  longer than device claims to support [len=126976] [max=65536A]

Signed-off-by: Niklas Söderlund 
Reported-by: Geert Uytterhoeven 

---

* Changes since v1
- Use devm_kzalloc() instead of a custom remove function to clean up.
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index c9dab9ce1ed55174..715e0c5dc30e4ebf 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -308,12 +308,23 @@ static const struct soc_device_attribute 
gen3_soc_whitelist[] = {
 static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
 {
const struct soc_device_attribute *soc = 
soc_device_match(gen3_soc_whitelist);
+   struct device *dev = >dev;
 
if (!soc)
return -ENODEV;
 
global_flags |= (unsigned long)soc->data;
 
+   if (!dev->dma_parms) {
+   dev->dma_parms = devm_kzalloc(dev, sizeof(*dev->dma_parms),
+ GFP_KERNEL);
+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+
+   if (dma_set_max_seg_size(dev, 0x))
+   dev_warn(dev, "Unable to set seg size\n");
+
return renesas_sdhi_probe(pdev, _sdhi_internal_dmac_dma_ops);
 }
 
-- 
2.18.0

Re: [PATCH] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

Hi Wolfram,

Thanks for your feedback.

On 2018-08-30 11:49:12 +0200, Wolfram Sang wrote:
> Hi Niklas,
> 
> > +   if (!dev->dma_parms) {
> > +   dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> 
> Can't we use devm_kzalloc and skip the custom remove function?

We could do that but then we can't reset the struct device dma_parms 
member to NULL.

kfree(dev->dma_parms);
dev->dma_parms = NULL;

This might not be a problem but I chose to opt for the more safer 
approach at first. If you or someone else thinks this is not an issue 
I'm more then happy to switch to devm_kcalloc().

-- 
Regards,
Niklas Söderlund

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Jacopo and Laurent,

On 2018-08-28 13:40:34 +0300, Laurent Pinchart wrote:

[snip]

> > >>> - The media_device_ops or at least the .link_notify() callback 
> > >>> of that
> > >>>   struct must be changed so not one driver in the media graph is
> > >>>   responsible for all links. In this case rcar-vin provides the
> > >>>   callback and rcar-vin should not judge which links between the
> > >>>   adv748x subdevices are OK to enable/disable. Currently the links
> > >>>   between the adv748x subdevices are immutably enabled to avoid this
> > >>>   particular problem.
> > >> 
> > >> Uh, I didn't get this :/ Care to elaborate more?
> > > 
> > > I'm thinking if it is not enough to just provide a .link_setup()
> > > callback to the (enabled) csi-2 sink pads of the adv748x and handle
> > > routing between the afe/hdmi sources and that sink, without the vin's
> > > registered link_notify playing any role in that.
> > 
> > That is my point, the v4l2 framework needs work to allow all drivers to
> > provide a .link_setup() callback. And this as you describe will conflict
> > with the current solution where there is only one possible such
> > callback. So in addition to being able to have multiple such callbacks
> > the current drivers implementing one would need to be updated to ignore
> > links which it do not care about.
> 
> Isn't this already possible ? We have a single top-level .link_notify() 
> operation at the media device level, but also .link_setup() operations for 
> each entity.

You are both right this is possible today, my bad. I had confused 
link_notify() and link_setup() and thought they where the same and that 
there could be only one implementation in struct media_device_ops which 
handled all link changes.

I'm happy to be proven wrong here as it will allow the adv748x to change 
links between its subdevices :-) Jacopo thanks for being persistent 
here!

-- 
Regards,
Niklas Söderlund

[PATCH v4 1/3] mmc: core: add helper to see if a host is doing a retune

[Niklas Söderlund]

Add a helper to allow host drivers checking if a retune is in progress.

Signed-off-by: Niklas Söderlund 
---
 include/linux/mmc/host.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index beed7121c7818b74..2a5fe75dd0821d03 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -569,6 +569,11 @@ static inline bool mmc_can_retune(struct mmc_host *host)
return host->can_retune == 1;
 }
 
+static inline bool mmc_doing_retune(struct mmc_host *host)
+{
+   return host->doing_retune == 1;
+}
+
 static inline enum dma_data_direction mmc_get_dma_dir(struct mmc_data *data)
 {
return data->flags & MMC_DATA_WRITE ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
-- 
2.18.0

[PATCH v4 2/3] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

From: Masaharu Hayakawa 

Checking for SCC error during retuning is unnecessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: fix small style issue]
Signed-off-by: Niklas Söderlund 

---

* Changes since v3
- Use the newly introduce helper mmc_doing_retune() instead of checking
  host->mmc->doing_retune in the host driver.

* Changes since v2
- Add comment to describe why checking SCC errors when using 4 taps is
  not needed.

* Changes since v1
- Use intermediate variable use_4tap to simplify check.
---
 drivers/mmc/host/renesas_sdhi_core.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index c2c0e444cfc9f2dc..360777e9f35cbb3e 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -500,6 +500,19 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host 
*host)
 static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
 {
struct renesas_sdhi *priv = host_to_priv(host);
+   bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
+
+   /*
+* Skip checking SCC errors when running on 4 taps in HS400 mode as
+* any retuning would still result in the same 4 taps being used.
+*/
+   if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))
+   return false;
+
+   if (mmc_doing_retune(host->mmc))
+   return false;
 
/* Check SCC error */
if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
-- 
2.18.0

[PATCH v4 0/3] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[Niklas Söderlund]

Hi,

These patches triggers a retune if a SCC error is detected. They where
ported from the renesas BSP. Masaharu-san did all the real work I just
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where
needed to adapt them to the recent HS400 series picked-up. It's tested
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200.

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

Niklas Söderlund (1):
  mmc: core: add helper to see if a host is doing a retune

 drivers/mmc/host/renesas_sdhi_core.c | 13 +
 drivers/mmc/host/tmio_mmc_core.c |  4 ++--
 include/linux/mmc/host.h |  5 +
 3 files changed, 20 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH v4 3/3] mmc: tmio: Fix SCC error detection

[Niklas Söderlund]

From: Masaharu Hayakawa 

SDR104, HS200 and HS400 need to check for SCC error. If SCC error is
detected, retuning is necessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund 

---

* Changes since v3
- Updated commit message to mention also HS400.
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index d48cd0402087ab31..f05c3a622f090cd6 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -839,8 +839,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
*host)
if (mrq->cmd->error || (mrq->data && mrq->data->error))
tmio_mmc_abort_dma(host);
 
-   if (host->check_scc_error)
-   host->check_scc_error(host);
+   if (host->check_scc_error && host->check_scc_error(host))
+   mrq->cmd->error = -EILSEQ;
 
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0

[PATCH] mmc: renesas_sdhi_internal_dmac: set scatter/gather max segment size

[Niklas Söderlund]

Fix warning when running with CONFIG_DMA_API_DEBUG_SG=y by allocating a
device_dma_parameters structure and filling in the max segment size. The
size used is the result of a discussion with Renesas hardware engineers
and unfortunately not found in the datasheet.

  renesas_sdhi_internal_dmac ee14.sd: DMA-API: mapping sg segment
  longer than device claims to support [len=126976] [max=65536A]

Signed-off-by: Niklas Söderlund 
Reported-by: Geert Uytterhoeven 
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 25 ++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 225af5389599108a..13a286e2a247042d 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -292,15 +292,38 @@ static const struct soc_device_attribute 
gen3_soc_whitelist[] = {
 static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
 {
const struct soc_device_attribute *soc = 
soc_device_match(gen3_soc_whitelist);
+   struct device *dev = >dev;
 
if (!soc)
return -ENODEV;
 
global_flags |= (unsigned long)soc->data;
 
+   if (!dev->dma_parms) {
+   dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
+   if (!dev->dma_parms)
+   return -ENOMEM;
+   }
+
+   if (dma_set_max_seg_size(dev, 0x))
+   dev_warn(dev, "Unable to set seg size\n");
+
return renesas_sdhi_probe(pdev, _sdhi_internal_dmac_dma_ops);
 }
 
+static int renesas_sdhi_internal_dmac_remove(struct platform_device *pdev)
+{
+   struct device *dev = >dev;
+   int ret;
+
+   ret = renesas_sdhi_remove(pdev);
+
+   kfree(dev->dma_parms);
+   dev->dma_parms = NULL;
+
+   return ret;
+}
+
 static const struct dev_pm_ops renesas_sdhi_internal_dmac_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
@@ -316,7 +339,7 @@ static struct platform_driver 
renesas_internal_dmac_sdhi_driver = {
.of_match_table = renesas_sdhi_internal_dmac_of_match,
},
.probe  = renesas_sdhi_internal_dmac_probe,
-   .remove = renesas_sdhi_remove,
+   .remove = renesas_sdhi_internal_dmac_remove,
 };
 
 module_platform_driver(renesas_internal_dmac_sdhi_driver);
-- 
2.18.0

Re: [PATCH] arm64: dts: renesas: r8a77965: Move timer node

[Niklas Söderlund]

Hi Geert,

Thanks for your patch.

On 2018-08-28 16:14:44 +0200, Geert Uytterhoeven wrote:
> To preserve alphabetical sort order.
> 
> Fixes: 4c529600eef0a6b7 ("arm64: dts: renesas: r8a77965: Add R-Car Gen3 
> thermal support")
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Niklas Söderlund 

> ---
>  arch/arm64/boot/dts/renesas/r8a77965.dtsi | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi 
> b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> index 776928518c9b0011..b14a1bad461648ab 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi
> @@ -1932,14 +1932,6 @@
>   };
>   };
>  
> - timer {
> - compatible = "arm,armv8-timer";
> - interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> -   < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> -   < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> -   < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>;
> - };
> -
>   thermal-zones {
>   sensor_thermal1: sensor-thermal1 {
>   polling-delay-passive = <250>;
> @@ -1984,6 +1976,14 @@
>   };
>   };
>  
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts-extended = < GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> +   < GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> +   < GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>,
> +   < GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(2) 
> | IRQ_TYPE_LEVEL_LOW)>;
> + };
> +
>   /* External USB clocks - can be overridden by the board */
>   usb3s0_clk: usb3s0 {
>   compatible = "fixed-clock";
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

Hi Wolfram,

Thanks for your feedback,

On 2018-08-29 09:16:04 +0200, Wolfram Sang wrote:
> 
> > +   /*
> > +* Skip checking SCC errors when running on 4 taps in HS400 mode as
> > +* any retuning would still result in the same 4 taps being used.
> > +*/
> 
> Did you verify this? I didn't have the time to do so :(
> 

I tested it on M3-W ES1.0 and it H3 ES2.0 and it works as I expect it to 
do, but forcing a retune due to SCC error is beyond my skill-set. I 
checked the documentation and can't find anything contradicting 
Shimoda-sans information from the HW team. So to the best of my 
knowledge this is correct. On the other hand I can't find anything in 
the documentation which supports the claim from the HW team.

If you have any tips on how to better test this I'm more then happy to 
do the work. Also would you like me to post a v4 addressing Ulf's 
comment or wait until we are convinced this is the right path forward?

-- 
Regards,
Niklas Söderlund

Re: Regarding VIN test(cvbs capture:- PAL resolution)

[Niklas Söderlund]

Hi Biju,

On 2018-08-29 10:41:16 +, Biju Das wrote:
> Hi All,
> 
> I started testing vin on R-Car Gen3(CVBS  input from DVD player connected to 
> R-Car M3-W,kernel:-renesas-devel-20180827-4.19-rc1)
> based on the information present in https://elinux.org/R-Car/Tests:rcar-vin.
> 
> Looks like PAL is not supported.
> When i execute the command, "media-ctl -d /dev/media1 --get-v4l2 "'adv748x 
> 4-0070 afe':8"" on target
> , i get "[fmt:UYVY8_2X8/720x240 field:alternate]" instead of 
> "[fmt:UYVY8_2X8/720x288 field:alternate]"
> 
> root@salvator-x:~# media-ctl -d /dev/media1 --get-v4l2 "'adv748x 4-0070 
> afe':8"
> [   28.708110] ##afe->curr_norm=1000 V4L2_STD_525_60=f900
> [fmt:UYVY8_2X8/720x240 field:alternate]
> 
> Q1) Have any one observed this issue?

The format from the AFE is based on the video standard set. As v4l2 
don't allow a subdevice to autodetect and change the video standard 
automatically, you need to do this as the first step of the pipeline 
configuration. The default standard of the adv748x driver is NTSC and 
that is why this step is not strictly needed when testing a NTSC source.


1. Boot board video source off and check formats

$ v4l2-ctl --get-detected-standard -d /dev/v4l-subdev23
Video Standard = 0x

$ media-ctl -d /dev/media2 --get-v4l2 "'adv748x 4-0070 afe':8"
[fmt:UYVY8_2X8/720x240 field:alternate colorspace:smpte170m]

2. Turn on video PAL source and check formats


$ v4l2-ctl --get-detected-standard -d /dev/v4l-subdev23
Video Standard = 0x00ff
PAL-B/B1/G/H/I/D/D1

$ media-ctl -d /dev/media2 --get-v4l2 "'adv748x 4-0070 afe':8"
[fmt:UYVY8_2X8/720x240 field:alternate colorspace:smpte170m]

Notice that the resolution is still for NTSC.

3. Set video standard of the AFE and check format

$ v4l2-ctl --set-standard=0x00ff -d /dev/v4l-subdev23
Standard set to 80ff

$ media-ctl -d /dev/media2 --get-v4l2 "'adv748x 4-0070 afe':8"
[fmt:UYVY8_2X8/720x288 field:alternate colorspace:smpte170m]

You now have a correct standard for the AFE and can configure a 
correct pipeline to capture PAL.

> 
> So I created a patch[2], based on [1]. With this, I get proper PAL 
> resolution(720x576)
> 
> root@salvator-x:~# media-ctl -d /dev/media1 --get-v4l2 "'adv748x 4-0070 
> afe':8"
> [   42.472582] ##afe->curr_norm=ff V4L2_STD_525_60=f900
> [fmt:UYVY8_2X8/720x288 field:alternate]
> 
> Q2) Is there any reason for not upstreaming the patch [1]?

As Kieran stated a subdevice is not allowed to change the video standard 
on it's own. Instead the user is required to ask the subevice which 
standard it detects and explicitly set that standard when configuring 
the video pipeline before capturing.

> 
> [1] 
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/horms/renesas-bsp/+/e0740949ae6b964da8bf1e88b504405276652aa7%5E%21/#F0
> 
> [2]
> --- a/drivers/media/i2c/adv748x/adv748x-afe.c
> +++ b/drivers/media/i2c/adv748x/adv748x-afe.c
> 
> @ -352,6 +353,7 @@ static int adv748x_afe_get_format(struct v4l2_subdev *sd,
> {
> struct adv748x_afe *afe = adv748x_sd_to_afe(sd);
> struct v4l2_mbus_framefmt *mbusformat;
> +   v4l2_std_id std = 0;
> /* It makes no sense to get the format of the analog sink pads */
> if (sdformat->pad != ADV748X_AFE_SOURCE)
> @@ -361,6 +363,9 @@ static int adv748x_afe_get_format(struct v4l2_subdev *sd,
> mbusformat = v4l2_subdev_get_try_format(sd, cfg, 
> sdformat->pad);
> sdformat->format = *mbusformat;
> } else {
> +   /* Set std_id automatically */
> +   adv748x_afe_querystd(sd, );
> +   adv748x_afe_s_std(sd, std);
> adv748x_afe_fill_format(afe, >format);
>     adv748x_afe_propagate_pixelrate(afe);
> 
> 
> Regards,
> Biju
> 
> 
> 
> 
> Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
> Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
> No. 04586709.

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

[Niklas Söderlund]

Hi Geert,

Thanks for your feedback.

On 2018-08-06 21:55:17 +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Mon, Aug 6, 2018 at 9:43 PM Niklas Söderlund
>  wrote:
> > Latest datasheet makes it clear that not all ES revisions of the H3 and
> > M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
> > unconditionally for these two SoCs. Prepare to handle the quirk based on
> > SoC revision instead of compatibility value by using soc_device_match()
> > and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.
> 
> Thanks for your patch!
> 
> > The reason for adding a new quirks struct instead of just a flag is that
> > looking ahead it seems more quirks needs to be handled in a SoC revision
> > basis.
> 
> Do you expect to add a lot? If yes...

Looking at the BSP it looks like there will be just one more quirk 
needed.

> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> 
> > @@ -569,6 +588,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> >
> > of_data = of_device_get_match_data(>dev);
> >
> > +   attr = soc_device_match(sdhi_quirks_match);
> > +   if (attr)
> > +   quirks = attr->data;
> 
> ... you may consider just storing a plain struct renesas_sdhi_of_data in
> the sdhi_quirks_match[] table.

I think this design is less messy as the quirks gets added in 
renesas_sdhi_core.c instead of needing to keep 
renesas_sdhi_internal_dmac.c and renesas_sdhi_sys_dmac.c in sync with 
the quirks.

Wolfram what do you think about this design?

> 
> > +
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!res)
> > return -EINVAL;
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds

-- 
Regards,
Niklas Söderlund

[PATCH v3 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

From: Masaharu Hayakawa 

Checking for SCC error during retuning is unnecessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: fix small style issue]
Signed-off-by: Niklas Söderlund 

---

* Changes since v2
- Add comment to describe why checking SCC errors when using 4 taps is
  not needed.

* Changes since v1
- Use intermediate variable use_4tap to simplify check.
---
 drivers/mmc/host/renesas_sdhi_core.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 777e32b0e410e850..4427f0e7058f3ee5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -443,6 +443,19 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host 
*host)
 static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
 {
struct renesas_sdhi *priv = host_to_priv(host);
+   bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
+
+   /*
+* Skip checking SCC errors when running on 4 taps in HS400 mode as
+* any retuning would still result in the same 4 taps being used.
+*/
+   if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))
+   return false;
+
+   if (host->mmc->doing_retune)
+   return false;
 
/* Check SCC error */
if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
-- 
2.18.0

[PATCH v3 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[Niklas Söderlund]

Hi,

These patches triggers a retune if a SCC error is detected. They where
ported from the renesas BSP. Masaharu-san did all the real work I just
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where
needed to adapt them to the recent HS400 series picked-up. It's tested
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200.

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

 drivers/mmc/host/renesas_sdhi_core.c | 13 +
 drivers/mmc/host/tmio_mmc_core.c |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH v3 2/2] mmc: tmio: Fix SCC error detection

[Niklas Söderlund]

From: Masaharu Hayakawa 

SDR104 and HS200 need to check for SCC error. If SCC error is detected,
retuning is necessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 261b4d62d2b1061f..268c4a3b728c775f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
*host)
if (mrq->cmd->error || (mrq->data && mrq->data->error))
tmio_mmc_abort_dma(host);
 
-   if (host->check_scc_error)
-   host->check_scc_error(host);
+   if (host->check_scc_error && host->check_scc_error(host))
+   mrq->cmd->error = -EILSEQ;
 
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Jacopo,

On 2018-08-27 11:49:56 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> A few more talk on the adv748x link handling, please bear with me...
> 
> On Sat, Aug 25, 2018 at 03:18:06PM +0200, jacopo mondi wrote:
> > Hi Laurent, Niklas,
> >
> > On Sat, Aug 25, 2018 at 08:37:15AM +0200, Niklas Söderlund wrote:
> > > Hi Laurent and Jacopo,
> > >
> > > On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> > > > Hi Jacopo,
> > > >
> > > > On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > > > > Hello renesas list,
> > > > >this series add supports for the HDMI and CVBS input to R-Car E3 
> > > > > R8A77990
> > > > > Ebisu board.
> > > > >
> > > > > It's an RFT, as I don't have an Ebisu to test with :(
> > > > >
> > > > > The series adds supports for the following items:
> > > > >
> > > > > - PFC: add VIN groups and functions
> > > > > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > > > > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > > > > - Ebisu: describe HDMI and CVBS inputs
> > > > >
> > > > > Each patch, when relevant reports difference between the upported BSP 
> > > > > patch
> > > > > and the proposed one.
> > > > >
> > > > > I know Laurent should receive an Ebisu sooner or later, maybe we can 
> > > > > sync
> > > > > for testing :)
> > > >
> > > > I've given the series a first test, and I think a bit more work is 
> > > > needed :-)
> > > >
> > > > [1.455533] adv748x 0-0070: Endpoint 
> > > > /soc/i2c@e650/video-receiver@70/
> > > > port@7/endpoint on port 7
> > > > [1.464683] adv748x 0-0070: Endpoint 
> > > > /soc/i2c@e650/video-receiver@70/
> > > > port@8/endpoint on port 8
> > > > [1.473728] adv748x 0-0070: Endpoint 
> > > > /soc/i2c@e650/video-receiver@70/
> > > > port@a/endpoint on port 10
> > > > [1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> > > > [1.639470] adv748x 0-0070: No endpoint found for txb
> > > > [1.644653] adv748x 0-0070: Failed to probe TXB
> > >
> > > I fear this is a design choice in the adv748x driver. Currently the
> > > driver requires both of its two CSI-2 transmitters to be connected/used
> > > else probe fails. Furthermore the HDMI capture is always routed to TXA
> > > while the analog capture is always routed to TXB.
> > >
> > > Now that we have a board where only TXA is connected but both HDMI and
> > > analog captures are used maybe it's time to do some more work on v4l2
> > > and the adv748x driver ;-P What's missing:
> > >
> > > - Probe should be OK with either TXA or TXB connected and not bail if
> > >   not both are used.
> >
> > I have three patches for this I hope to share as soon as I'll be able
> > to do some more testing
> >
> > > - The media_device_ops or at least the .link_notify() callback of that
> > >   struct must be changed so not one driver in the media graph is
> > >   responsible for all links. In this case rcar-vin provides the callback
> > >   and rcar-vin should not judge which links between the adv748x
> > >   subdevices are OK to enable/disable. Currently the links between the
> > >   adv748x subdevices are immutably enabled to avoid this particular
> > >   problem.
> >
> > Uh, I didn't get this :/ Care to elaborate more?
> >
> 
> I'm thinking if it is not enough to just provide a .link_setup()
> callback to the (enabled) csi-2 sink pads of the adv748x and handle
> routing between the afe/hdmi sources and that sink, without the vin's
> registered link_notify playing any role in that.

That is my point, the v4l2 framework needs work to allow all drivers to 
provide a .link_setup() callback. And this as you describe will conflict 
with the current solution where there is only one possible such 
callback. So in addition to being able to have multiple such callbacks 
the current drivers implementing one would need to be updated to ignore 
links which it do not care about.

In our case the .link_setup() in rcar-vin should not care about the 
links between the adv748x internal subdevice. Of course the other way 
around is also true, the .link_setup() in adv748x should not care about 
the links handled by rcar-vin :-)

As you discovers this is not possi

Re: [RFT 0/8] arm64: dts: renesas: Ebisu: Add HDMI and CVBS input

[Niklas Söderlund]

Hi Laurent and Jacopo,

On 2018-08-25 02:54:44 +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> On Monday, 20 August 2018 13:16:34 EEST Jacopo Mondi wrote:
> > Hello renesas list,
> >this series add supports for the HDMI and CVBS input to R-Car E3 R8A77990
> > Ebisu board.
> > 
> > It's an RFT, as I don't have an Ebisu to test with :(
> > 
> > The series adds supports for the following items:
> > 
> > - PFC: add VIN groups and functions
> > - R-Car VIN and R-Car CSI-2: add support for R8A77990
> > - R8A77990: Add I2C, VIN and CSI-2 nodes
> > - Ebisu: describe HDMI and CVBS inputs
> > 
> > Each patch, when relevant reports difference between the upported BSP patch
> > and the proposed one.
> > 
> > I know Laurent should receive an Ebisu sooner or later, maybe we can sync
> > for testing :)
> 
> I've given the series a first test, and I think a bit more work is needed :-)
> 
> [1.455533] adv748x 0-0070: Endpoint /soc/i2c@e650/video-receiver@70/
> port@7/endpoint on port 7
> [1.464683] adv748x 0-0070: Endpoint /soc/i2c@e650/video-receiver@70/
> port@8/endpoint on port 8
> [1.473728] adv748x 0-0070: Endpoint /soc/i2c@e650/video-receiver@70/
> port@a/endpoint on port 10
> [1.484835] adv748x 0-0070: chip found @ 0xe0 revision 2143
> [1.639470] adv748x 0-0070: No endpoint found for txb
> [1.644653] adv748x 0-0070: Failed to probe TXB

I fear this is a design choice in the adv748x driver. Currently the 
driver requires both of its two CSI-2 transmitters to be connected/used 
else probe fails. Furthermore the HDMI capture is always routed to TXA 
while the analog capture is always routed to TXB.

Now that we have a board where only TXA is connected but both HDMI and 
analog captures are used maybe it's time to do some more work on v4l2 
and the adv748x driver ;-P What's missing:

- Probe should be OK with either TXA or TXB connected and not bail if 
  not both are used.
- The media_device_ops or at least the .link_notify() callback of that 
  struct must be changed so not one driver in the media graph is 
  responsible for all links. In this case rcar-vin provides the callback 
  and rcar-vin should not judge which links between the adv748x 
  subdevices are OK to enable/disable. Currently the links between the 
  adv748x subdevices are immutably enabled to avoid this particular 
  problem.

> 
> > PS: the list of upported patches will be sent separately.
> > 
> > Jacopo Mondi (5):
> >   media: dt-bindings: media: rcar-vin: Add R8A77990 support
> >   media: rcar-vin: Add support for R-Car R8A77990
> >   dt-bindings: media: rcar-csi2: Add R8A77990
> >   media: rcar-csi2: Add R8A77990 support
> >   arm64: dts: renesas: ebisu: Add HDMI and CVBS input
> > 
> > Koji Matsuoka (1):
> >   arm64: dts: r8a77990: Add VIN and CSI-2 device nodes
> > 
> > Takeshi Kihara (2):
> >   pinctrl: sh-pfc: r8a77990: Add VIN pins, groups and functions
> >   arm64: dts: r8a77990: Add I2C device nodes
> > 
> >  .../devicetree/bindings/media/rcar_vin.txt |   1 +
> >  .../bindings/media/renesas,rcar-csi2.txt   |   1 +
> >  arch/arm64/boot/dts/renesas/r8a77990-ebisu.dts |  86 
> >  arch/arm64/boot/dts/renesas/r8a77990.dtsi  | 202 +
> >  drivers/media/platform/rcar-vin/rcar-core.c|  20 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c|   9 +
> >  drivers/pinctrl/sh-pfc/pfc-r8a77990.c  | 504 ++
> >  7 files changed, 823 insertions(+)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 1/4] vin-tests: Add support for D3 Draak

[Niklas Söderlund]

Hi Jacopo,

On 2018-08-24 18:28:49 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Fri, Aug 24, 2018 at 06:12:29PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-08-24 12:24:19 +0200, Jacopo Mondi wrote:
> > > Add support for D3 Draak board.
> > >
> > > Draak has its HDMI input connected to an ADV7612 which connects to VIN4
> > > parallel data inputs.
> > >
> > > Signed-off-by: Jacopo Mondi 
> > > ---
> > >  scripts/boards.sh | 9 +
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/scripts/boards.sh b/scripts/boards.sh
> > > index 26fdab9..7eb3a27 100644
> > > --- a/scripts/boards.sh
> > > +++ b/scripts/boards.sh
> > > @@ -29,6 +29,13 @@ case $info in
> > >  # for V3M, but results in an image.
> > >  parallelformat="YUYV8_1X16"
> > >  ;;
> > > +"Renesas Draak board based on r8a77995")
> > > +gen="gen3"
> > > +vins="4"
> > > +parallelname="adv7612 0-004c"
> > > +# FIXME: This is a hackfor D3, but results in an image.
> > > +parallelformat="YUYV8_1X16"
> > > +;;
> > >  "Koelsch")
> > >  gen="gen2"
> > >
> > > @@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
> > >
> > >  txaname="adv748x 0-0070 txa"
> > >  txbname="adv748x 0-0070 txb"
> > > +elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
> > > +hdminame="adv7612 0-004c"
> >
> > I don't have the D3 schematics at hand but this feels wrong. Why do you
> > define the adv7612 as both a parallel and CSI-2 source? IIRC the D3 have
> > no CSI-2 IP?
> >
> 
> Why is "hdminame" variable the CSI-2 source? It might as well be the
> VIN source, as for D3.

Well the design is currently that the parallel source is named 
'parallelname'. I agree that the name hdminame is not veary good in this 
context but it came from the time when a parallel source was not 
supported by the driver and the 'parallelname' variable was added once 
V3M support where added.

> 
> To me, and I have added it here because it is used in yavta-hdmi in
> this way, is the HDMI input component.

Correct yavta-hdmi uses only 'hdminame' variable as that script do not 
(yet) support parallel video source, only test-qv4l2.sh do this at the 
moment.

> 
> Want to change the name?
> 
> > The reason this exists for V3M is that it has both parallel input in the
> > form of a adv7612 and a CSI-2 input in the for of a adv7482 connected to
> > the CSI40 IP.
> >
> > >  else
> > >  cvbsname="adv748x 4-0070 afe"
> > >  hdminame="adv748x 4-0070 hdmi"
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund



-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 4/4] vin-tests: yavta-hdmi: Add VIN4 and parallel link

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-08-24 12:24:22 +0200, Jacopo Mondi wrote:
> Add support for VIN4 to yavta-hdmi and check if format propagation should
> go through 'mc_propagate_parallel()' if the HDMI receiver chip is an
> ADV7612 one.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  yavta-hdmi | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/yavta-hdmi b/yavta-hdmi
> index fdec546..2e3b625 100755
> --- a/yavta-hdmi
> +++ b/yavta-hdmi
> @@ -33,14 +33,23 @@ case $vc in
>  dev=/dev/$vin3
>  csipad=4
>  ;;
> +4)
> +vinname=$vinname4
> +dev=/dev/$vin4

I think you should also add a csipad declaration here as if the script 
is used on a board which is not D3 VIN4 would be connected to a CSI-2 
bus. Writing that I realise a new var 'csidev' or something would be 
needed here to expand this to cover the full range of VIN0-VIN7.

> +;;
>  *)
>  echo "Unkown VC '$vc'"
>  exit 1
>  esac
>  
>  mc_reset
> -mc_set_link "$csi40name" $csipad "$vinname" 1
> -mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad 
> "$vinname"
> +if [[ "$hdminame" == "adv7612 0-004c" ]]; then


You should use $parallelname here not $hdminame. Furthermore I thin you 
should check if the variable is empty or not and not target it for a 
specific board.

A good (or only) example of how I think this should be done can be found 
in test-qv4l2.sh.

 if [[ "$csi20name" != "" ]]; then
 mc_set_link "$csi20name" 1 "$vinname1" 1
 mc_propagate_cvbs "$vinname1"
 qv4l2 -d /dev/$vin1
 fi

 if [[ "$parallelname" != "" ]]; then
 mc_reset
 mc_set_link "$parallelname" 1 "$vinname0" 1
 mc_propagate_parallel "$vinname0"
 qv4l2 -d /dev/$vin0
 fi

> + mc_set_link "$hdminame" 1  "$vinname" 1
> + mc_propagate_parallel "$vinname"
> +else
> + mc_set_link "$csi40name" $csipad "$vinname" 1
> + mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad 
> "$vinname"
> +fi
>  
>  out=/tmp/vin-tests
>  rm -fr $out
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 3/4] vin-tests: Add capture format for parallel input

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-08-24 12:24:21 +0200, Jacopo Mondi wrote:
> Add configurable capture format to propagate_parallel() function.
> The capture format is the image format set on the VIN nodes.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  scripts/boards.sh| 2 ++
>  scripts/vin-tests.sh | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/boards.sh b/scripts/boards.sh
> index 7eb3a27..b379af6 100644
> --- a/scripts/boards.sh
> +++ b/scripts/boards.sh
> @@ -28,6 +28,7 @@ case $info in
>  # FIXME: This is a hack and not the correct mbus format
>  # for V3M, but results in an image.
>  parallelformat="YUYV8_1X16"
> + parallel_captureformat="RGB565"

I'm sorry I don't see the value of this change, am I missing something?  
There is no functional change but I assume you use this for something 
but until I figure out what I will leave this change hanging.

>  ;;
>  "Renesas Draak board based on r8a77995")
>  gen="gen3"
> @@ -35,6 +36,7 @@ case $info in
>  parallelname="adv7612 0-004c"
>  # FIXME: This is a hackfor D3, but results in an image.
>  parallelformat="YUYV8_1X16"
> + parallel_captureformat="RGB565"
>  ;;
>  "Koelsch")
>  gen="gen2"
> diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
> index 0c5b29a..e7b7a48 100644
> --- a/scripts/vin-tests.sh
> +++ b/scripts/vin-tests.sh
> @@ -111,5 +111,5 @@ mc_propagate_parallel() {
>  echo "format: $format size: $size/$vinsize field: $field/$vinfield vdev: 
> $vdev"
>  
>  $mediactl -d $mdev -V "$cam [fmt:$format/$size field:$field]"
> -yavta -f RGB565 -s $vinsize --field $vinfield $vdev
> +yavta -f $parallel_captureformat -s $vinsize --field $vinfield $vdev
>  }
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 2/4] vin-tests: Fix vdev name in propagate_parallel

[Niklas Söderlund]

Hi Jacopo,

Thanks for finding this bug.

On 2018-08-24 12:24:20 +0200, Jacopo Mondi wrote:
> The $vdev variable was not defined in propagate_parallel() function.
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Niklas Söderlund 

Applied to my tree.

> ---
>  scripts/vin-tests.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
> index ebb1477..0c5b29a 100644
> --- a/scripts/vin-tests.sh
> +++ b/scripts/vin-tests.sh
> @@ -83,6 +83,7 @@ mc_propagate_cvbs() {
>  }
>  
>  mc_propagate_parallel() {
> +vin="$1"
>  mdev=$(mc_get_mdev)
>  
>  cam="'$parallelname':1"
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 1/4] vin-tests: Add support for D3 Draak

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-08-24 12:24:19 +0200, Jacopo Mondi wrote:
> Add support for D3 Draak board.
> 
> Draak has its HDMI input connected to an ADV7612 which connects to VIN4
> parallel data inputs.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  scripts/boards.sh | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/boards.sh b/scripts/boards.sh
> index 26fdab9..7eb3a27 100644
> --- a/scripts/boards.sh
> +++ b/scripts/boards.sh
> @@ -29,6 +29,13 @@ case $info in
>  # for V3M, but results in an image.
>  parallelformat="YUYV8_1X16"
>  ;;
> +"Renesas Draak board based on r8a77995")
> +gen="gen3"
> +vins="4"
> +parallelname="adv7612 0-004c"
> +# FIXME: This is a hackfor D3, but results in an image.
> +parallelformat="YUYV8_1X16"
> +;;
>  "Koelsch")
>  gen="gen2"
>  
> @@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
>  
>  txaname="adv748x 0-0070 txa"
>  txbname="adv748x 0-0070 txb"
> +elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
> +hdminame="adv7612 0-004c"

I don't have the D3 schematics at hand but this feels wrong. Why do you 
define the adv7612 as both a parallel and CSI-2 source? IIRC the D3 have 
no CSI-2 IP?

The reason this exists for V3M is that it has both parallel input in the 
form of a adv7612 and a CSI-2 input in the for of a adv7482 connected to 
the CSI40 IP.

>  else
>  cvbsname="adv748x 4-0070 afe"
>  hdminame="adv748x 4-0070 hdmi"
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/3] i2c: sh_mobile: fix leak when using DMA bounce buffer

[Niklas Söderlund]

Hi Wolfram,

Thanks for your work.

On 2018-08-24 16:52:46 +0200, Wolfram Sang wrote:
> We only freed the bounce buffer after successful DMA, missing the cases
> where DMA setup may have gone wrong. Use a better location which always
> gets called after each message and use 'stop_after_dma' as a flag for a
> successful transfer.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/i2c/busses/i2c-sh_mobile.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
> b/drivers/i2c/busses/i2c-sh_mobile.c
> index 456581e3c1d2..c56a86da958e 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -515,8 +515,6 @@ static void sh_mobile_i2c_dma_callback(void *data)
>   pd->pos = pd->msg->len;
>   pd->stop_after_dma = true;
>  
> - i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg, true);
> -
>   iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
>  }
>  
> @@ -714,6 +712,10 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter 
> *adapter,
>   timeout = wait_event_timeout(pd->wait,
>  pd->sr & (ICSR_TACK | SW_DONE),
>  adapter->timeout);
> +
> + /* 'stop_after_dma' tells if DMA transfer was complete */
> + i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg, 
> pd->stop_after_dma);
> +
>   if (!timeout) {
>   dev_err(pd->dev, "Transfer request timed out\n");
>   if (pd->dma_direction != DMA_NONE)
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/3] i2c: sh_mobile: define start_ch() void as it only returns 0 anyhow

[Niklas Söderlund]

Hi Wolfram,

Thanks for your work.

On 2018-08-24 16:52:45 +0200, Wolfram Sang wrote:
> After various refactoring over the years, start_ch() doesn't return
> errno anymore, so make the function return void. This saves the error
> handling when calling it which in turn eases cleanup of resources of a
> future patch.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/i2c/busses/i2c-sh_mobile.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
> b/drivers/i2c/busses/i2c-sh_mobile.c
> index 16c35d474398..456581e3c1d2 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -610,8 +610,8 @@ static void sh_mobile_i2c_xfer_dma(struct 
> sh_mobile_i2c_data *pd)
>   dma_async_issue_pending(chan);
>  }
>  
> -static int start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
> - bool do_init)
> +static void start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
> +  bool do_init)
>  {
>   if (do_init) {
>   /* Initialize channel registers */
> @@ -635,7 +635,6 @@ static int start_ch(struct sh_mobile_i2c_data *pd, struct 
> i2c_msg *usr_msg,
>  
>   /* Enable all interrupts to begin with */
>   iic_wr(pd, ICIC, ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> - return 0;
>  }
>  
>  static int poll_dte(struct sh_mobile_i2c_data *pd)
> @@ -706,9 +705,7 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter *adapter,
>   pd->send_stop = i == num - 1 || msg->flags & I2C_M_STOP;
>   pd->stop_after_dma = false;
>  
> - err = start_ch(pd, msg, do_start);
> - if (err)
> - break;
> + start_ch(pd, msg, do_start);
>  
>   if (do_start)
>   i2c_op(pd, OP_START, 0);
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/3] i2c: refactor function to release a DMA safe buffer

[Niklas Söderlund]

Hi Wolfram,

Thanks for your patch.

On 2018-08-24 16:52:44 +0200, Wolfram Sang wrote:
> a) rename to 'put' instead of 'release' to match 'get' when obtaining
>the buffer
> b) change the argument order to have the buffer as first argument
> c) add a new argument telling the function if the message was
>transferred. This allows the function to be used also in cases
>where setting up DMA failed, so the buffer needs to be freed without
>syncing to the message buffer.
> 
> Also convert the only user.
> 
> Signed-off-by: Wolfram Sang 

Reviewed-by: Niklas Söderlund 

> ---
>  Documentation/i2c/DMA-considerations | 10 +++---
>  drivers/i2c/busses/i2c-sh_mobile.c   |  2 +-
>  drivers/i2c/i2c-core-base.c  | 11 ++-
>  include/linux/i2c.h  |  2 +-
>  4 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/i2c/DMA-considerations 
> b/Documentation/i2c/DMA-considerations
> index 966610aa4620..bf40f5c01d15 100644
> --- a/Documentation/i2c/DMA-considerations
> +++ b/Documentation/i2c/DMA-considerations
> @@ -50,10 +50,14 @@ bounce buffer. But you don't need to care about that 
> detail, just use the
>  returned buffer. If NULL is returned, the threshold was not met or a bounce
>  buffer could not be allocated. Fall back to PIO in that case.
>  
> -In any case, a buffer obtained from above needs to be released. It ensures 
> data
> -is copied back to the message and a potentially used bounce buffer is freed::
> +In any case, a buffer obtained from above needs to be released. Another 
> helper
> +function ensures a potentially used bounce buffer is freed::
>  
> - i2c_release_dma_safe_msg_buf(msg, dma_buf);
> + i2c_put_dma_safe_msg_buf(dma_buf, msg, xferred);
> +
> +The last argument 'xferred' controls if the buffer is synced back to the
> +message or not. No syncing is needed in cases setting up DMA had an error and
> +there was no data transferred.
>  
>  The bounce buffer handling from the core is generic and simple. It will 
> always
>  allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c 
> b/drivers/i2c/busses/i2c-sh_mobile.c
> index 9c7f6f8ceb22..16c35d474398 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -515,7 +515,7 @@ static void sh_mobile_i2c_dma_callback(void *data)
>   pd->pos = pd->msg->len;
>   pd->stop_after_dma = true;
>  
> - i2c_release_dma_safe_msg_buf(pd->msg, pd->dma_buf);
> + i2c_put_dma_safe_msg_buf(pd->dma_buf, pd->msg, true);
>  
>   iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
>  }
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5a937109a289..4e381aeff917 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2302,21 +2302,22 @@ u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, 
> unsigned int threshold)
>  EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
>  
>  /**
> - * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> i2c_msg
> - * @msg: the message to be synced with
> + * i2c_put_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
>   * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> + * @msg: the message which the buffer corresponds to
> + * @xferred: bool saying if the message was transferred
>   */
> -void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> +void i2c_put_dma_safe_msg_buf(u8 *buf, struct i2c_msg *msg, bool xferred)
>  {
>   if (!buf || buf == msg->buf)
>   return;
>  
> - if (msg->flags & I2C_M_RD)
> + if (xferred && msg->flags & I2C_M_RD)
>   memcpy(msg->buf, buf, msg->len);
>  
>   kfree(buf);
>  }
> -EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
> +EXPORT_SYMBOL_GPL(i2c_put_dma_safe_msg_buf);
>  
>  MODULE_AUTHOR("Simon G. Vogl ");
>  MODULE_DESCRIPTION("I2C-Bus main module");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 36f357ecdf67..6f77708544ff 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -861,7 +861,7 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
> i2c_msg *msg)
>  }
>  
>  u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> -void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
> +void i2c_put_dma_safe_msg_buf(u8 *buf, struct i2c_msg *msg, bool xferred);
>  
>  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
> addr);
>  /**
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

[PATCH 2/2] mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

[Niklas Söderlund]

It was though all ES revisions of H3 and M3-W SoCs required the
TMIO_MMC_HAVE_4TAP_HS400 flag. Recent datasheet updates tells us this is
not true, only early ES revisions of the SoC do.

Since quirk matching based on ES revisions is now used to handle the
flag it's possible to align all Gen3 compatibility properties. This will
allow later ES revisions of H3 and M3-W to use the correct 8-tap HS400
mode.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++-
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 17 ++--
 2 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c 
b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 35cc0de6be67a515..d032bd63444d1029 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -82,22 +82,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-   /* DMAC can handle 0x blk count but only 1 segment */
-   .max_blk_count  = 0x,
-   .max_segs   = 1,
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -114,8 +98,8 @@ static const struct renesas_sdhi_of_data 
of_rcar_gen3_compatible = {
 };
 
 static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
{},
 };
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c 
b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 890f192dedbdcc9c..4bb46c489d71f848 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -78,19 +78,6 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
},
 };
 
-static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = {
-   .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
- TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 |
- TMIO_MMC_HAVE_4TAP_HS400,
-   .capabilities   = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
- MMC_CAP_CMD23,
-   .capabilities2  = MMC_CAP2_NO_WRITE_PROTECT,
-   .bus_shift  = 2,
-   .scc_offset = 0x1000,
-   .taps   = rcar_gen3_scc_taps,
-   .taps_num   = ARRAY_SIZE(rcar_gen3_scc_taps),
-};
-
 static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
@@ -117,8 +104,8 @@ static const struct of_device_id 
renesas_sdhi_sys_dmac_of_match[] = {
{ .compatible = "renesas,sdhi-r8a7792", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7793", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,sdhi-r8a7794", .data = 
_rcar_gen2_compatible, },
-   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_r8a7795_compatible, },
-   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_r8a7795_compatible, },
+   { .compatible = "renesas,sdhi-r8a7795", .data = 
_rcar_gen3_compatible, },
+   { .compatible = "renesas,sdhi-r8a7796", .data = 
_rcar_gen3_compatible, },
{ .compatible = "renesas,rcar-gen1-sdhi", .data = 
_rcar_gen1_compatible, },
{ .compatible = "renesas,rcar-gen2-sdhi", .data = 
_rcar_gen2_compatible, },
{ .compatible = "renesas,rcar-gen3-sdhi", .data = 
_rcar_gen3_compatible, },
-- 
2.18.0

[PATCH 0/2] mmc: renesas_sdhi: extend quirk selection to handle ES revisions

[Niklas Söderlund]

Hi,

Recent datasheet updates have made it clear that some quirks are not SoC 
specific but SoC + ES version specific. Currently the quirks are 
selected using compatibility values but whit this new information that 
is not enough.

This small series adds support to select quirks based on SoC + ES 
revision using soc_device_match(). It handles the only quirk upstreamed 
so far, selection of 4-taps for HS400 mode on R-CarH3(ES1.0,ES2.0) and 
R-CarM3W(ES1.0,ES1.1).

As this solution makes the compatible selection method obsolete it is 
removed and all Gen3 compat values now points to the same data 
structure. The specific compats for H3 and M3-W are however kept in the 
driver for backward compatibility.

Based on mmc/next and tested on H3 ES2.0 and M3-N.

Niklas Söderlund (2):
  mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision
  mmc: renesas_sdhi: align compatibility properties for H3 and M3-W

 drivers/mmc/host/renesas_sdhi_core.c  | 26 +++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 20 ++
 drivers/mmc/host/renesas_sdhi_sys_dmac.c  | 17 ++--
 3 files changed, 30 insertions(+), 33 deletions(-)

-- 
2.18.0

[PATCH 1/2] mmc: renesas_sdhi: handle 4tap hs400 mode quirk based on SoC revision

[Niklas Söderlund]

Latest datasheet makes it clear that not all ES revisions of the H3 and
M3-W have the 4-tap HS400 mode quirk, currently the quirk is set
unconditionally for these two SoCs. Prepare to handle the quirk based on
SoC revision instead of compatibility value by using soc_device_match()
and set the TMIO_MMC_HAVE_4TAP_HS400 flag explicitly.

The reason for adding a new quirks struct instead of just a flag is that
looking ahead it seems more quirks needs to be handled in a SoC revision
basis.

Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/renesas_sdhi_core.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 66e90f233cefcba5..781a31592330f872 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "renesas_sdhi.h"
 #include "tmio_mmc.h"
@@ -48,6 +49,10 @@
 #define SDHI_VER_GEN3_SD   0xcc10
 #define SDHI_VER_GEN3_SDMMC0xcd10
 
+struct renesas_sdhi_quirks {
+   bool hs400_4taps;
+};
+
 static void renesas_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
 {
u32 val;
@@ -555,11 +560,25 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host 
*host, bool enable)
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const struct renesas_sdhi_quirks sdhi_quirks_h3_m3w = {
+   .hs400_4taps = true,
+};
+
+static const struct soc_device_attribute sdhi_quirks_match[]  = {
+   { .soc_id = "r8a7795", .revision = "ES1.*", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7795", .revision = "ES2.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.0", .data = _quirks_h3_m3w 
},
+   { .soc_id = "r8a7796", .revision = "ES1.1", .data = _quirks_h3_m3w 
},
+   { /* Sentinel. */ },
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
   const struct tmio_mmc_dma_ops *dma_ops)
 {
struct tmio_mmc_data *mmd = pdev->dev.platform_data;
+   const struct renesas_sdhi_quirks *quirks = NULL;
const struct renesas_sdhi_of_data *of_data;
+   const struct soc_device_attribute *attr;
struct tmio_mmc_data *mmc_data;
struct tmio_mmc_dma *dma_priv;
struct tmio_mmc_host *host;
@@ -569,6 +588,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
of_data = of_device_get_match_data(>dev);
 
+   attr = soc_device_match(sdhi_quirks_match);
+   if (attr)
+   quirks = attr->data;
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -EINVAL;
@@ -634,6 +657,9 @@ int renesas_sdhi_probe(struct platform_device *pdev,
host->multi_io_quirk= renesas_sdhi_multi_io_quirk;
host->dma_ops   = dma_ops;
 
+   if (quirks && quirks->hs400_4taps)
+   mmc_data->flags |= TMIO_MMC_HAVE_4TAP_HS400;
+
/* For some SoC, we disable internal WP. GPIO may override this */
if (mmc_can_gpio_ro(host->mmc))
mmc_data->capabilities2 &= ~MMC_CAP2_NO_WRITE_PROTECT;
-- 
2.18.0

[PATCH v2 2/2] mmc: tmio: Fix SCC error detection

[Niklas Söderlund]

From: Masaharu Hayakawa 

SDR104 and HS200 need to check for SCC error. If SCC error is detected,
retuning is necessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 261b4d62d2b1061f..268c4a3b728c775f 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -919,8 +919,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
*host)
if (mrq->cmd->error || (mrq->data && mrq->data->error))
tmio_mmc_abort_dma(host);
 
-   if (host->check_scc_error)
-   host->check_scc_error(host);
+   if (host->check_scc_error && host->check_scc_error(host))
+   mrq->cmd->error = -EILSEQ;
 
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0

[PATCH v2 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[Niklas Söderlund]

Hi,

These patches triggers a retune if a SCC error is detected. They where
ported from the renesas BSP. Masaharu-san did all the real work I just
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where
needed to adapt them to the recent HS400 series picked-up. It's tested
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

 drivers/mmc/host/renesas_sdhi_core.c | 9 +
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.18.0

[PATCH v2 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

From: Masaharu Hayakawa 

Checking for SCC error during retuning is unnecessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: fix small style issue]
Signed-off-by: Niklas Söderlund 

---

* Changes since v1
- Use intermediate variable use_4tap to simplify check.
---
 drivers/mmc/host/renesas_sdhi_core.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c 
b/drivers/mmc/host/renesas_sdhi_core.c
index 777e32b0e410e850..66e90f233cefcba5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -443,6 +443,15 @@ static int renesas_sdhi_select_tuning(struct tmio_mmc_host 
*host)
 static bool renesas_sdhi_check_scc_error(struct tmio_mmc_host *host)
 {
struct renesas_sdhi *priv = host_to_priv(host);
+   bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;
+
+   if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
+   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))
+   return false;
+
+   if (host->mmc->doing_retune)
+   return false;
 
/* Check SCC error */
if (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_RVSCNTL) &
-- 
2.18.0

Re: [PATCH] thermal: rcar_gen3_thermal: convert to SPDX identifiers

[Niklas Söderlund]

Hi Morimoto-san,

Thanks for your patch.

On 2018-07-30 07:57:22 +, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto 
> 
> Signed-off-by: Kuninori Morimoto 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/thermal/rcar_gen3_thermal.c | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/thermal/rcar_gen3_thermal.c 
> b/drivers/thermal/rcar_gen3_thermal.c
> index 766521e..7aed533 100644
> --- a/drivers/thermal/rcar_gen3_thermal.c
> +++ b/drivers/thermal/rcar_gen3_thermal.c
> @@ -1,19 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   *  R-Car Gen3 THS thermal sensor driver
>   *  Based on rcar_thermal.c and work from Hien Dang and Khiem Nguyen.
>   *
>   * Copyright (C) 2016 Renesas Electronics Corporation.
>   * Copyright (C) 2016 Sang Engineering
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; version 2 of the License.
> - *
> - *  This program is distributed in the hope that it will be useful, but
> - *  WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - *  General Public License for more details.
> - *
>   */
>  #include 
>  #include 
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] mmc: renesas_sdhi: skip SCC error check when retuning

[Niklas Söderlund]

Hi Wolfram,

On 2018-07-25 14:21:07 +0200, Wolfram Sang wrote:
> Hi Niklas,
> 
> > * Changes since v3
> > - Add check for 4TAP for HS400.
> 
> Is it the same in the BSP or where does this info come from?

It comes from the BSP but I had to modify it to fit with the upstream 
implementation of 4 vs 8 taps. The original code from BSP:

if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
!(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
!(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 &&
  !host->hs400_use_4tap))
return false;
> 
> 
> > +   if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
> > +   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
> > +   !(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 &&
> > + !(host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400)))
> 
> This becomes very hard to read. We need to simplify it.

I agree but I could not find a neat way of doing it. How about?

bool use_4tap = host->pdata->flags & TMIO_MMC_HAVE_4TAP_HS400;

if (!(host->mmc->ios.timing == MMC_TIMING_UHS_SDR104) &&
!(host->mmc->ios.timing == MMC_TIMING_MMC_HS200) &&
!(host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && !use_4tap))
   return false;

> 
> And can you bounce me your debugging mail from today again? I seem to
> have lost it :(

Bounced.

> 
> Thanks,
> 
>Wolfram



-- 
Regards,
Niklas Söderlund

[PATCH 2/2] mmc: tmio: Fix SCC error detection

[Niklas Söderlund]

From: Masaharu Hayakawa 

SDR104 and HS200 need to check for SCC error. If SCC error is detected,
retuning is necessary.

Signed-off-by: Masaharu Hayakawa 
[Niklas: update commit message]
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 72ac806e0c762d83..81883bc08524e441 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -920,8 +920,8 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host 
*host)
if (mrq->cmd->error || (mrq->data && mrq->data->error))
tmio_mmc_abort_dma(host);
 
-   if (host->check_scc_error)
-   host->check_scc_error(host);
+   if (host->check_scc_error && host->check_scc_error(host))
+   mrq->cmd->error = -EILSEQ;
 
/* If SET_BLOCK_COUNT, continue with main command */
if (host->mrq && !mrq->cmd->error) {
-- 
2.18.0

[PATCH 0/2] mmc: {tmio,renesas_sdhi}: retune if SCC error detected

[Niklas Söderlund]

Hi,

These patches triggers a retune if a SCC error is detected. They where 
ported from the renesas BSP. Masaharu-san did all the real work I just 
ported them to upstream and did small fixups.

These patches where broken out of my retuning series as more work where 
needed to adapt them to the recent HS400 series picked-up. It's tested 
on M3-N using both HS200 and HS400 and on H3 ES2.0 using HS200.

Masaharu Hayakawa (2):
  mmc: renesas_sdhi: skip SCC error check when retuning
  mmc: tmio: Fix SCC error detection

 drivers/mmc/host/renesas_sdhi_core.c | 9 +
 drivers/mmc/host/tmio_mmc_core.c | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.18.0