Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
> 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. Less code churn sounds good to me. Thanks, Niklas! signature.asc Description: PGP signature
Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
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 v2] mmc: tmio: Add initial setting of interrupt mask register
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... signature.asc Description: PGP signature
Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
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
Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
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? signature.asc Description: PGP signature
Re: [PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
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 Reviewed-by: Simon Horman
[PATCH v2] mmc: tmio: Add initial setting of interrupt mask register
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
