diff mbox

[RESEND,PATCHv2,1/3] arm: socfpga: Set the SDMMC clock phase in system manager

Message ID 1381780051-1826-1-git-send-email-dinguyen@altera.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dinh Nguyen Oct. 14, 2013, 7:47 p.m. UTC
From: Dinh Nguyen <dinguyen@altera.com>

Add functionality in the System Manager to set the SDR settings for the
SD/MMC IP.

Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
Cc: Pavel Machek <pavel@denx.de>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Olof Johansson <olof@lixom.net>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Chris Ball <cjb@laptop.org>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Seungwon Jeon <tgih.jun@samsung.com>
Cc: devicetree@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org

---
v2:
- Have socfpga_sysmgr_set_dwmmc_drvsel_smpsel() take in the SDR settings
directly so that it does not have to walk the DTB.
---
 arch/arm/mach-socfpga/Makefile     |    2 +-
 arch/arm/mach-socfpga/core.h       |    6 ++++++
 arch/arm/mach-socfpga/system_mgr.c |   28 ++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-socfpga/system_mgr.c

Comments

Jaehoon Chung Oct. 15, 2013, 6:51 a.m. UTC | #1
Hi Dinh,

On 10/15/2013 04:47 AM, dinguyen@altera.com wrote:
> From: Dinh Nguyen <dinguyen@altera.com>
> 
> Add functionality in the System Manager to set the SDR settings for the
> SD/MMC IP.
> 
> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
> Cc: Pavel Machek <pavel@denx.de>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Olof Johansson <olof@lixom.net>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> Cc: Seungwon Jeon <tgih.jun@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-mmc@vger.kernel.org
> CC: linux-arm-kernel@lists.infradead.org
> 
> ---
> v2:
> - Have socfpga_sysmgr_set_dwmmc_drvsel_smpsel() take in the SDR settings
> directly so that it does not have to walk the DTB.
> ---
>  arch/arm/mach-socfpga/Makefile     |    2 +-
>  arch/arm/mach-socfpga/core.h       |    6 ++++++
>  arch/arm/mach-socfpga/system_mgr.c |   28 ++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-socfpga/system_mgr.c
> 
> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
> index 6dd7a93..e4ff8b9 100644
> --- a/arch/arm/mach-socfpga/Makefile
> +++ b/arch/arm/mach-socfpga/Makefile
> @@ -2,5 +2,5 @@
>  # Makefile for the linux kernel.
>  #
>  
> -obj-y					:= socfpga.o
> +obj-y					:= socfpga.o system_mgr.o
>  obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
> diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
> index 572b8f7..b05fa6a 100644
> --- a/arch/arm/mach-socfpga/core.h
> +++ b/arch/arm/mach-socfpga/core.h
> @@ -44,4 +44,10 @@ extern unsigned long cpu1start_addr;
>  
>  #define SOCFPGA_SCU_VIRT_BASE   0xfffec000
>  
> +/* SDMMC Group for System Manager defines */
> +#define SYSMGR_SDMMCGRP_CTRL_OFFSET             0x108
> +#define DRV_CLK_PHASE_SHIFT_SEL_MASK    0x7
I didn't see where this define is used. When do this use?

Best Regards,
Jaehoon Chung

> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)          \
> +	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
> +
>  #endif
> diff --git a/arch/arm/mach-socfpga/system_mgr.c b/arch/arm/mach-socfpga/system_mgr.c
> new file mode 100644
> index 0000000..9fdce1d
> --- /dev/null
> +++ b/arch/arm/mach-socfpga/system_mgr.c
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright Altera Corporation (C) 2013. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/of_platform.h>
> +#include <asm/mach/map.h>
> +
> +#include "core.h"
> +
> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
> +{
> +	u32 hs_timing;
> +
> +	hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
> +	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
> +}
> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dinh Nguyen Oct. 15, 2013, 12:37 p.m. UTC | #2
Hi Jaehoo,

On 10/15/13 1:51 AM, Jaehoon Chung wrote:
> Hi Dinh,
>
> On 10/15/2013 04:47 AM, dinguyen@altera.com wrote:
>> From: Dinh Nguyen <dinguyen@altera.com>
>>
>> Add functionality in the System Manager to set the SDR settings for the
>> SD/MMC IP.
>>
>> Signed-off-by: Dinh Nguyen <dinguyen@altera.com>
>> Cc: Pavel Machek <pavel@denx.de>
>> CC: Arnd Bergmann <arnd@arndb.de>
>> CC: Olof Johansson <olof@lixom.net>
>> Cc: Rob Herring <rob.herring@calxeda.com>
>> Cc: Pawel Moll <pawel.moll@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Stephen Warren <swarren@wwwdotorg.org>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Chris Ball <cjb@laptop.org>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> Cc: Seungwon Jeon <tgih.jun@samsung.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-mmc@vger.kernel.org
>> CC: linux-arm-kernel@lists.infradead.org
>>
>> ---
>> v2:
>> - Have socfpga_sysmgr_set_dwmmc_drvsel_smpsel() take in the SDR settings
>> directly so that it does not have to walk the DTB.
>> ---
>>   arch/arm/mach-socfpga/Makefile     |    2 +-
>>   arch/arm/mach-socfpga/core.h       |    6 ++++++
>>   arch/arm/mach-socfpga/system_mgr.c |   28 ++++++++++++++++++++++++++++
>>   3 files changed, 35 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/mach-socfpga/system_mgr.c
>>
>> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
>> index 6dd7a93..e4ff8b9 100644
>> --- a/arch/arm/mach-socfpga/Makefile
>> +++ b/arch/arm/mach-socfpga/Makefile
>> @@ -2,5 +2,5 @@
>>   # Makefile for the linux kernel.
>>   #
>>   
>> -obj-y					:= socfpga.o
>> +obj-y					:= socfpga.o system_mgr.o
>>   obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
>> diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
>> index 572b8f7..b05fa6a 100644
>> --- a/arch/arm/mach-socfpga/core.h
>> +++ b/arch/arm/mach-socfpga/core.h
>> @@ -44,4 +44,10 @@ extern unsigned long cpu1start_addr;
>>   
>>   #define SOCFPGA_SCU_VIRT_BASE   0xfffec000
>>   
>> +/* SDMMC Group for System Manager defines */
>> +#define SYSMGR_SDMMCGRP_CTRL_OFFSET             0x108
>> +#define DRV_CLK_PHASE_SHIFT_SEL_MASK    0x7
> I didn't see where this define is used. When do this use?
It's not used at all. I will remove.

Thanks for the review.
Dinh
>
> Best Regards,
> Jaehoon Chung
>
>> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)          \
>> +	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
>> +
>>   #endif
>> diff --git a/arch/arm/mach-socfpga/system_mgr.c b/arch/arm/mach-socfpga/system_mgr.c
>> new file mode 100644
>> index 0000000..9fdce1d
>> --- /dev/null
>> +++ b/arch/arm/mach-socfpga/system_mgr.c
>> @@ -0,0 +1,28 @@
>> +/*
>> + * Copyright Altera Corporation (C) 2013. All Rights Reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + *
>> + * You should have received a copy of the GNU General Public License along with
>> + * this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#include <linux/of_platform.h>
>> +#include <asm/mach/map.h>
>> +
>> +#include "core.h"
>> +
>> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
>> +{
>> +	u32 hs_timing;
>> +
>> +	hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
>> +	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
>> +}
>> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Oct. 15, 2013, 12:50 p.m. UTC | #3
On Monday 14 October 2013, dinguyen@altera.com wrote:
> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
> +{
> +       u32 hs_timing;
> +
> +       hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
> +       writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
> +}
> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);

This looks like the wrong approach. What are you trying to do? If you want to
set a clock, please use the clk API.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dinh Nguyen Oct. 15, 2013, 1:22 p.m. UTC | #4
Hi Arnd,

On 10/15/13 7:50 AM, Arnd Bergmann wrote:
> On Monday 14 October 2013, dinguyen@altera.com wrote:
>> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
>> +{
>> +       u32 hs_timing;
>> +
>> +       hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
>> +       writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
>> +}
>> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
> This looks like the wrong approach. What are you trying to do? If you want to
> set a clock, please use the clk API.
I can't use the clk API because this function is setting up a clock 
phase bit for the SD/MMC
clock that is used to clock the card, not the IP. This register is 
located outside the SD/MMC
and the clock manager.

Just to refresh your memory on this topic:
I tried to use the syscon approach that you suggested:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168470.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/170423.html

But this approach was rejected by Stephen Warren because we wanted to 
the SD driver to be automonous
of registers outside its IP:

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/194014.html

So I went with the approach of exposing a platform API so that the 
SD/MMC platform specific
code can call it.

The system manager has a plethora of registers that controls other IPs 
on the SOC, so I kinda thought
syscon was the way to go with this. A driver for this IP did not make 
sense to me.

Please advise if you know of another approach?

Thanks,
Dinh






>
> 	Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Oct. 15, 2013, 7:01 p.m. UTC | #5
On Tuesday 15 October 2013, Dinh Nguyen wrote:
> Hi Arnd,
> 
> On 10/15/13 7:50 AM, Arnd Bergmann wrote:
> > On Monday 14 October 2013, dinguyen@altera.com wrote:
> >> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
> >> +{
> >> +       u32 hs_timing;
> >> +
> >> +       hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
> >> +       writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
> >> +}
> >> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
> > This looks like the wrong approach. What are you trying to do? If you want to
> > set a clock, please use the clk API.
> I can't use the clk API because this function is setting up a clock 
> phase bit for the SD/MMC
> clock that is used to clock the card, not the IP. This register is 
> located outside the SD/MMC
> and the clock manager.
> 
> Just to refresh your memory on this topic:
> I tried to use the syscon approach that you suggested:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168470.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/170423.html

Ah, thanks. I knew this problem had come up before, I just didn't remember
it was for socfpga.

> But this approach was rejected by Stephen Warren because we wanted to 
> the SD driver to be automonous
> of registers outside its IP:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/194014.html
> 
> So I went with the approach of exposing a platform API so that the 
> SD/MMC platform specific
> code can call it.
> 
> The system manager has a plethora of registers that controls other IPs 
> on the SOC, so I kinda thought
> syscon was the way to go with this. A driver for this IP did not make 
> sense to me.
> 
> Please advise if you know of another approach?

I don't remember the details of what we have gone through before, but
I think this should still work:

1 Create a "syscon" backend driver to control your "system manager", which
  lets other drivers hook into it without calling a private API.
2 Create a trivial clock driver that is independent of your existing
  clock driver and independent of the other drivers using the system
  manager, by using syscon as the low-level interface.
3 Make the sdmmc driver use the normal clock API and link its clock to the
  driver from step 2 in the device tree.

Is this what you have tried before?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dinh Nguyen Oct. 15, 2013, 7:19 p.m. UTC | #6
Hi Arnd,

On 10/15/13 2:01 PM, Arnd Bergmann wrote:
> On Tuesday 15 October 2013, Dinh Nguyen wrote:
>> Hi Arnd,
>>
>> On 10/15/13 7:50 AM, Arnd Bergmann wrote:
>>> On Monday 14 October 2013, dinguyen@altera.com wrote:
>>>> +void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
>>>> +{
>>>> +       u32 hs_timing;
>>>> +
>>>> +       hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
>>>> +       writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
>>>> +}
>>>> +EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);
>>> This looks like the wrong approach. What are you trying to do? If you want to
>>> set a clock, please use the clk API.
>> I can't use the clk API because this function is setting up a clock
>> phase bit for the SD/MMC
>> clock that is used to clock the card, not the IP. This register is
>> located outside the SD/MMC
>> and the clock manager.
>>
>> Just to refresh your memory on this topic:
>> I tried to use the syscon approach that you suggested:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/168470.html
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-May/170423.html
> Ah, thanks. I knew this problem had come up before, I just didn't remember
> it was for socfpga.
>
>> But this approach was rejected by Stephen Warren because we wanted to
>> the SD driver to be automonous
>> of registers outside its IP:
>>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-August/194014.html
>>
>> So I went with the approach of exposing a platform API so that the
>> SD/MMC platform specific
>> code can call it.
>>
>> The system manager has a plethora of registers that controls other IPs
>> on the SOC, so I kinda thought
>> syscon was the way to go with this. A driver for this IP did not make
>> sense to me.
>>
>> Please advise if you know of another approach?
> I don't remember the details of what we have gone through before, but
> I think this should still work:
>
> 1 Create a "syscon" backend driver to control your "system manager", which
>    lets other drivers hook into it without calling a private API.
Yes, if you look at drivers/mmc/host/dw_mmc-socfpga.c that is in the 
mainline,
it is hooking into the "system manager" through "syscon". Is this what you
mean here?

The problem is because of the SYSMGR_SDMMCGRP_CTRL_OFFSET define
in this file. This means the SD/MMC driver needs information that is 
outside of its IP.

Dinh
> 2 Create a trivial clock driver that is independent of your existing
>    clock driver and independent of the other drivers using the system
>    manager, by using syscon as the low-level interface.
> 3 Make the sdmmc driver use the normal clock API and link its clock to the
>    driver from step 2 in the device tree.
>
> Is this what you have tried before?
>
> 	Arnd

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Oct. 15, 2013, 7:47 p.m. UTC | #7
On Tuesday 15 October 2013, Dinh Nguyen wrote:
> >
> > 1 Create a "syscon" backend driver to control your "system manager", which
> >    lets other drivers hook into it without calling a private API.
> Yes, if you look at drivers/mmc/host/dw_mmc-socfpga.c that is in the 
> mainline,
> it is hooking into the "system manager" through "syscon". Is this what you
> mean here?

No, because you directly hook into the syscon driver, rather than using
a clock driver as the middle-man, see steps 2 and 3 below.
 
> The problem is because of the SYSMGR_SDMMCGRP_CTRL_OFFSET define
> in this file. This means the SD/MMC driver needs information that is 
> outside of its IP.

Yes, this is not ideal because you don't really want that information
in the sd/mmc driver. That driver should only know about the fact
that it talks to a clock controller, not how it's implemented.

	Arnd

> > 2 Create a trivial clock driver that is independent of your existing
> >    clock driver and independent of the other drivers using the system
> >    manager, by using syscon as the low-level interface.
> > 3 Make the sdmmc driver use the normal clock API and link its clock to the
> >    driver from step 2 in the device tree.
> >
> > Is this what you have tried before?

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dinh Nguyen Oct. 15, 2013, 8:21 p.m. UTC | #8
On 10/15/13 2:47 PM, Arnd Bergmann wrote:
> On Tuesday 15 October 2013, Dinh Nguyen wrote:
>>> 1 Create a "syscon" backend driver to control your "system manager", which
>>>     lets other drivers hook into it without calling a private API.
>> Yes, if you look at drivers/mmc/host/dw_mmc-socfpga.c that is in the
>> mainline,
>> it is hooking into the "system manager" through "syscon". Is this what you
>> mean here?
> No, because you directly hook into the syscon driver, rather than using
> a clock driver as the middle-man, see steps 2 and 3 below.
>   
Ok, now I understand.
>> The problem is because of the SYSMGR_SDMMCGRP_CTRL_OFFSET define
>> in this file. This means the SD/MMC driver needs information that is
>> outside of its IP.
> Yes, this is not ideal because you don't really want that information
> in the sd/mmc driver. That driver should only know about the fact
> that it talks to a clock controller, not how it's implemented.
>
> 	Arnd
>
>>> 2 Create a trivial clock driver that is independent of your existing
>>>     clock driver and independent of the other drivers using the system
>>>     manager, by using syscon as the low-level interface.
>>> 3 Make the sdmmc driver use the normal clock API and link its clock to the
>>>     driver from step 2 in the device tree.
This makes sense for the SD/MMC driver, but do you think I can use the 
same approach for
other drivers that this system manager touches? i.e. The ethernet IP's 
PHY setting is controlled
by a register that is in the system manager as well.

I have submitted this patch for enabling ethernet. It is making use of 
the driver platform specific
driver calls to touch the system manager.

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/200173.html

Just wondering if that is right approach for the ethernet driver? If 
not, then I think I may have
to come up with a generic system manager driver so that it can be used 
for other IPs.

Thanks,
Dinh

>>>
>>> Is this what you have tried before?

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Oct. 16, 2013, 6:56 p.m. UTC | #9
On Tuesday 15 October 2013, Dinh Nguyen wrote:
> This makes sense for the SD/MMC driver, but do you think I can use the 
> same approach for
> other drivers that this system manager touches?

It might not be as straightforward for every driver, but something like
this should work at least for some of the more interesting ones. For the
rest, you can have a system manager driver that exports functions.

> i.e. The ethernet IP's 
> PHY setting is controlled
> by a register that is in the system manager as well.
>
> I have submitted this patch for enabling ethernet. It is making use of 
> the driver platform specific
> driver calls to touch the system manager.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-September/200173.html

I definitely don't want to see auxdata getting used for stuff like this.

In case of the ethernet driver, I suspect an SoC specific PHY driver would
be the right approach.
 
> Just wondering if that is right approach for the ethernet driver? If 
> not, then I think I may have
> to come up with a generic system manager driver so that it can be used 
> for other IPs.

The general way of handling this should be to see if there is a subsystem
available for handling a particular feature. In case of clk and phy, we
have those subsystems, so it's a matter of writing an appropriate driver
that communicates with the high-level drivers using that.

I don't know what other features are provided by the system manager that
might need a different solution, but if you can't come up with a nice
solution, you can either have an soc specific portion in the device driver
(as you suggested for sdmmc first), or you can have that platform specific
mfd driver exporting some functions.
If the functionality is actually something common but doesn't have a
subsystem in Linux yet, the "right" approach might also be to introduce
a new subsystem, which I realize can be a lot of work.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
index 6dd7a93..e4ff8b9 100644
--- a/arch/arm/mach-socfpga/Makefile
+++ b/arch/arm/mach-socfpga/Makefile
@@ -2,5 +2,5 @@ 
 # Makefile for the linux kernel.
 #
 
-obj-y					:= socfpga.o
+obj-y					:= socfpga.o system_mgr.o
 obj-$(CONFIG_SMP)	+= headsmp.o platsmp.o
diff --git a/arch/arm/mach-socfpga/core.h b/arch/arm/mach-socfpga/core.h
index 572b8f7..b05fa6a 100644
--- a/arch/arm/mach-socfpga/core.h
+++ b/arch/arm/mach-socfpga/core.h
@@ -44,4 +44,10 @@  extern unsigned long cpu1start_addr;
 
 #define SOCFPGA_SCU_VIRT_BASE   0xfffec000
 
+/* SDMMC Group for System Manager defines */
+#define SYSMGR_SDMMCGRP_CTRL_OFFSET             0x108
+#define DRV_CLK_PHASE_SHIFT_SEL_MASK    0x7
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel)          \
+	((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
+
 #endif
diff --git a/arch/arm/mach-socfpga/system_mgr.c b/arch/arm/mach-socfpga/system_mgr.c
new file mode 100644
index 0000000..9fdce1d
--- /dev/null
+++ b/arch/arm/mach-socfpga/system_mgr.c
@@ -0,0 +1,28 @@ 
+/*
+ * Copyright Altera Corporation (C) 2013. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/of_platform.h>
+#include <asm/mach/map.h>
+
+#include "core.h"
+
+void socfpga_sysmgr_set_dwmmc_drvsel_smpsel(u32 drvsel, u32 smplsel)
+{
+	u32 hs_timing;
+
+	hs_timing = SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel);
+	writel(hs_timing, sys_manager_base_addr + SYSMGR_SDMMCGRP_CTRL_OFFSET);
+}
+EXPORT_SYMBOL(socfpga_sysmgr_set_dwmmc_drvsel_smpsel);