Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On Mon, Sep 12, 2016 at 10:15:05AM -0400, Chris Brandt wrote: > In moving platforms from board files to DT, there still needs to be a way > to set the ocr_mask setting for the tmio driver during probe. Without this > setting, the probe will fail because the supported voltages are not known. > > This patch will also traditional platform registration platforms to > migrate to DT. > > Signed-off-by: Chris Brandt I'm fine with this change: Reviewed-by: Wolfram Sang Maybe some details from this discussion could be added to the commit message? signature.asc Description: PGP signature
RE: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On 9/17/2016, Ulf Hansson wrote: > > I'm assuming I'll have to do the same for MMC (sh_mmcif.c) once I get to > that driver (I need to find my MMC PLUS card first for testing). > > Okay! > > Kind regards > Uffe Just FYI, For the eMMC case, I decided not to modify sh_mmcif.c to add the OCR and instead just create a 3.3 fixed regulator in the DT. The reason is that if someone is using eMMC for their file system, they are probably using external RAM (since block data needs to be copied out of eMMC into RAM before they can be executed). So the extra RAM needed for the regular DT is not really an issue. However, the people using SDHI are using it for SDIO for Wifi modules and they are the ones with the slimmed down RAM systems. So for them, removing the regulator DT will help squeeze their system into 3MB or 5MB of in-chip system RAM. Chris
Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On 13 September 2016 at 17:59, Chris Brandt wrote: > Hello Uffe > > Thank you for your reply. > > On 9/13/2016, Ulf Hansson wrote: >> Because, if you have an external regulator feeding the mmc with power you >> should set it up and use it. > > But in many board designs, there is no control over the regulator supply (or > you would rather have your bootloader do that, not the kernel). > > So what this patch is doing is setting it to a default of 3.3v (for just this > SoC) and if someone wants to add a regulator in the board's DT file, that > will override the ocr_mask set by the driver. That's perfectly okay to me! I just wanted to get clear picture of *why* this change was needed. > > > I'm assuming I'll have to do the same for MMC (sh_mmcif.c) once I get to that > driver (I need to find my MMC PLUS card first for testing). Okay! Kind regards Uffe
RE: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
Hello Uffe Thank you for your reply. On 9/13/2016, Ulf Hansson wrote: > Because, if you have an external regulator feeding the mmc with power you > should set it up and use it. But in many board designs, there is no control over the regulator supply (or you would rather have your bootloader do that, not the kernel). So what this patch is doing is setting it to a default of 3.3v (for just this SoC) and if someone wants to add a regulator in the board's DT file, that will override the ocr_mask set by the driver. I'm assuming I'll have to do the same for MMC (sh_mmcif.c) once I get to that driver (I need to find my MMC PLUS card first for testing). Chris
Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On 13 September 2016 at 15:50, Chris Brandt wrote: > Here's a question: > > The DT regulator method is good if you want to be able to control the > regulator at run-time by the system. > > But for MMC and SDHI, why isn't there a way to just set the OCR voltage in > the board's DT if it's fixed (other than making a fixed regulator node)? > Why isn't there a mmc-ocr-mask property? That's really what I want and it > seems like it would make a lot of dts files more simple. Because, if you have an external regulator feeding the mmc with power you should set it up and use it. Now, we do have cases where it's actually the MMC controller that manges the power to the card. So no external regulators being used. For these case we have the "voltage-ranges" DT binding, but I don't think you should be using that for these cases. :-) Kind regards Uffe
RE: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
Here's a question: The DT regulator method is good if you want to be able to control the regulator at run-time by the system. But for MMC and SDHI, why isn't there a way to just set the OCR voltage in the board's DT if it's fixed (other than making a fixed regulator node)? Why isn't there a mmc-ocr-mask property? That's really what I want and it seems like it would make a lot of dts files more simple. Chris
RE: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On 9/13/2016, Ulf Hansson wrote: > Regarding the ocr_mask; How do these SoCs provides the power to the mmc/sd > card? > > Do note, I am *not* talking about the I/O voltage but the core power to > the card. The SoC does not *provide* power to the card. It's not that fancy. How the 3.3v is supplied to the card slot is up to the board designer. But, the SoC spec says the I/O should be 3.3v, so we might as well fix the OCR as 3.3v because they don't have a choice. I did see all the DT regulator stuff for the other Renesas SoC, but all that is beyond how this chip is being used. Also note that this SoC (Renesas RZ/A1), unlike the others, is being used with XIP_KERNEL because it's possible to run a full kernel without any external memory (it comes in a 3MB, 5MB, or 10RM internal RAM versions). So, I'm trying to avoid adding extra DT "stuff" that just takes up more RAM resources and has no value for these simple embedded systems. I know in other DDR based system with Megs and Megs of RAM to spare this isn't a problem, but in an XIP_KERNEL environment, it all adds up. One of the reasons why I haven't been trying to push our BSP upstream and move everyone is because I'm afraid of what all the new DT bloat is going to do to the kernel RAM usage. Chris
Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
Hi Ulf,
On Tue, Sep 13, 2016 at 2:57 PM, Ulf Hansson wrote:
> On 12 September 2016 at 16:15, Chris Brandt wrote:
>> In moving platforms from board files to DT, there still needs to be a way
>> to set the ocr_mask setting for the tmio driver during probe. Without this
>> setting, the probe will fail because the supported voltages are not known.
>
> Regarding the ocr_mask; How do these SoCs provides the power to the mmc/sd
> card?
>
> Do note, I am *not* talking about the I/O voltage but the core power
> to the card.
>
> The reason for raising the question is that we have infrastructures in
> the mmc core which can create the ocr_mask, by parsing a regulator's
> voltage range. This is the recommended method to use, instead of using
> hard coded ocr mask values.
On RSKRZA1, 3.3V is provided to the SD/MMC socket through an MIC2026
MOSFET switch.
On Genmai, 3.3V or 5V is provided through an LTC1471 switch.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 1/3] mmc: sh_mobile_sdhi: add ocr_mask option
On 12 September 2016 at 16:15, Chris Brandt wrote:
> In moving platforms from board files to DT, there still needs to be a way
> to set the ocr_mask setting for the tmio driver during probe. Without this
> setting, the probe will fail because the supported voltages are not known.
Regarding the ocr_mask; How do these SoCs provides the power to the mmc/sd card?
Do note, I am *not* talking about the I/O voltage but the core power
to the card.
The reason for raising the question is that we have infrastructures in
the mmc core which can create the ocr_mask, by parsing a regulator's
voltage range. This is the recommended method to use, instead of using
hard coded ocr mask values.
Kind regards
Uffe
>
> This patch will also traditional platform registration platforms to
> migrate to DT.
>
> Signed-off-by: Chris Brandt
> ---
> drivers/mmc/host/sh_mobile_sdhi.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c
> b/drivers/mmc/host/sh_mobile_sdhi.c
> index 5334f24..b033500 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -59,6 +59,7 @@ enum tmio_mmc_dmac_type {
>
> struct sh_mobile_sdhi_of_data {
> unsigned long tmio_flags;
> + u32 tmio_ocr_mask;
> unsigned long capabilities;
> unsigned long capabilities2;
> enum dma_slave_buswidth dma_buswidth;
> @@ -630,6 +631,7 @@ static int sh_mobile_sdhi_probe(struct platform_device
> *pdev)
> const struct sh_mobile_sdhi_of_data *of_data = of_id->data;
>
> mmc_data->flags |= of_data->tmio_flags;
> + mmc_data->ocr_mask = of_data->tmio_ocr_mask;
> mmc_data->capabilities |= of_data->capabilities;
> mmc_data->capabilities2 |= of_data->capabilities2;
> mmc_data->dma_rx_offset = of_data->dma_rx_offset;
> --
> 2.9.2
>
>
