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

2018-09-20 Thread Wolfram Sang
On Thu, Sep 20, 2018 at 12:02:27AM +0200, Niklas Söderlund wrote:
> 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?

Yes, please.



signature.asc
Description: PGP signature


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

2018-09-19 Thread 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 1/3] mmc: tmio: Add initial setting of interrupt mask register

2018-09-16 Thread Wolfram Sang

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



signature.asc
Description: PGP signature


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

2018-06-27 Thread Niklas Söderlund
Hi Wolfram,

Thanks for your feedback.

On 2018-06-27 09:27:02 +0200, Wolfram Sang wrote:
> 
> > > > +#define TMIO_MASK_INIT  0x8b7f031d /* H/W initial value */
> > > 
> > > known on R-Car 2+ only!
> > 
> > No it's also know on Gen3 :-)
> 
> Sure, 2+ means "R-Car 2 and later". Like with GPL2+. Sorry if this was
> not clear. And it is still my main argument to do...

When you point it out it's clear, sorry I missed that.

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

-- 
Regards,
Niklas Söderlund


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

2018-06-27 Thread Wolfram Sang

> > > +#define TMIO_MASK_INIT  0x8b7f031d /* H/W initial value */
> > 
> > known on R-Car 2+ only!
> 
> No it's also know on Gen3 :-)

Sure, 2+ means "R-Car 2 and later". Like with GPL2+. Sorry if this was
not clear. And it is still my main argument to do...

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



signature.asc
Description: PGP signature


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

2018-06-27 Thread Niklas Söderlund
Hi Wolfram,

Thanks for your feedback.

On 2018-06-09 20:15:52 +0200, Wolfram Sang wrote:
> Hi Niklas,
> 
> On Thu, Jun 07, 2018 at 01:22:50AM +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 of initial program.
> 
> s/initial program/bootloader/ ?

Sounds much better.

> 
> > Since the error interrupts may be unmasked, the driver sets initial value.
> > 
> > Signed-off-by: Masaharu Hayakawa 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/mmc/host/tmio_mmc.h  | 1 +
> >  drivers/mmc/host/tmio_mmc_core.c | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index e7d651352dc90fb8..bfb76b2963f80894 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -102,6 +102,7 @@
> >  
> >  /* Define some IRQ masks */
> >  /* This is the mask used at reset by the chip */
> > +#define TMIO_MASK_INIT  0x8b7f031d /* H/W initial value */
> 
> known on R-Car 2+ only!

No it's also know on Gen3 :-)

If you check the documents for the registers SD_INFO1_MASK and 
SD_INFO2_MASK they describe there respective initial values. If one 
combines them you get the same constant as documented here.

The tricky part is that sd_ctrl_write32_as_16_and_16() is used to write 
them so you have to take 16 bits from each register description as that 
is how the are written. After the fun bus shift is applied this ends up 
writing the lower 16 bits of the constant to 0x80 and the higher 
16 of it to 0x88.

> 
> >  #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 3080299303045adc..2ede81476daf76ce 100644
> > --- a/drivers/mmc/host/tmio_mmc_core.c
> > +++ b/drivers/mmc/host/tmio_mmc_core.c
> > @@ -1273,6 +1273,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
> > tmio_mmc_clk_stop(_host);
> > tmio_mmc_reset(_host);
> >  
> > +   sd_ctrl_write32_as_16_and_16(_host, CTL_IRQ_MASK, TMIO_MASK_INIT);
> 
> 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.
> 
> Regards,
> 
>Wolfram
> 



-- 
Regards,
Niklas Söderlund


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

2018-06-06 Thread 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 of initial program.
Since the error interrupts may be unmasked, the driver sets initial value.

Signed-off-by: Masaharu Hayakawa 
Signed-off-by: Niklas Söderlund 
---
 drivers/mmc/host/tmio_mmc.h  | 1 +
 drivers/mmc/host/tmio_mmc_core.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index e7d651352dc90fb8..bfb76b2963f80894 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -102,6 +102,7 @@
 
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
+#define TMIO_MASK_INIT  0x8b7f031d /* H/W initial value */
 #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 3080299303045adc..2ede81476daf76ce 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1273,6 +1273,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
tmio_mmc_clk_stop(_host);
tmio_mmc_reset(_host);
 
+   sd_ctrl_write32_as_16_and_16(_host, CTL_IRQ_MASK, TMIO_MASK_INIT);
_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.17.0