Re: [PATCH] drivers: genpd: let platform code to register devices into disabled domains
On Thursday, May 10, 2012, Marek Szyprowski wrote:
> Hi Rafael,
>
> On Monday, May 07, 2012 8:45 PM Rafael J. Wysocki wrote:
>
> > On Monday, May 07, 2012, Marek Szyprowski wrote:
> > > Hi Rafael,
> > >
> > > I'm sorry for a late reply, I was on holidays last week and just got back
> > > to
> > > the office.
> > >
> > > On Sunday, April 29, 2012 10:55 PM Rafael J. Wysocki wrote:
> > >
> > > > On Friday, April 06, 2012, Marek Szyprowski wrote:
> > > > > Some bootloaders disable power domains on boot and the platform
> > > > > startup
> > > > > code registers them in the 'disabled' state. Current gen_pd code
> > > > > assumed
> > > > > that the devices can be registered only to the power domain which is
> > > > > in
> > > > > 'enabled' state and these devices are active at the time of the
> > > > > registration. This is not correct in our case. This patch lets drivers
> > > > > to be registered into 'disabled' power domains and finally solves
> > > > > mysterious freezes and lack of resume/suspend calls on Samsung Exynos4
> > > > > NURI and UniversalC210 platforms.
> > > > >
> > > > > Signed-off-by: Marek Szyprowski
> > > > > Signed-off-by: Kyungmin Park
> > > > > ---
> > > > > drivers/base/power/domain.c |7 +--
> > > > > 1 files changed, 1 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > > > > index 73ce9fb..05f5799f 100644
> > > > > --- a/drivers/base/power/domain.c
> > > > > +++ b/drivers/base/power/domain.c
> > > > > @@ -1211,11 +1211,6 @@ int __pm_genpd_add_device(struct
> > > > > generic_pm_domain *genpd, struct
> > > > device *dev,
> > > > >
> > > > > genpd_acquire_lock(genpd);
> > > > >
> > > > > - if (genpd->status == GPD_STATE_POWER_OFF) {
> > > > > - ret = -EINVAL;
> > > > > - goto out;
> > > > > - }
> > > > > -
> > > > > if (genpd->prepared_count > 0) {
> > > > > ret = -EAGAIN;
> > > > > goto out;
> > > > > @@ -1239,7 +1234,7 @@ int __pm_genpd_add_device(struct
> > > > > generic_pm_domain *genpd, struct
> > > > device *dev,
> > > > > dev_pm_get_subsys_data(dev);
> > > > > dev->power.subsys_data->domain_data = &gpd_data->base;
> > > > > gpd_data->base.dev = dev;
> > > > > - gpd_data->need_restore = false;
> > > > > + gpd_data->need_restore = true;
> > > >
> > > > I think that should be:
> > > >
> > > > + gpd_data->need_restore = genpd->status == GPD_STATE_POWER_OFF;
> > > >
> > > > Otherwise, on the next domain power off the device's state won't be
> > > > saved.
> > > >
> > > > > list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
> > > > > if (td)
> > > > > gpd_data->td = *td;
> > >
> > > I've tested the above change and there is problem. Let me explain in
> > > detail the
> > > sw/hw configuration I have.
> > >
> > > There is a power domain and the device driver. The device itself also has
> > > it's own
> > > power management code (which enables and disables clocks). Some power
> > > domains are
> > > disabled by bootloader and some devices in the active power domains have
> > > their
> > > clocks disabled too. In the current runtime pm code the devices were
> > > probed in
> > > 'disabled' state and had to enable itself by calling get_runtime_sync().
> > > My initial
> > > patch restored runtime pm handling to the old state (the same which was
> > > with non
> > > gen_pd based driver or no power domain driver at all, where runtime pm
> > > was handled
> > > by platform bus). If I apply your patch the runtime_restore
> >
> > I guess you mean .runtime_resume().
> >
> > > callback is not called on first driver probe for devices inside the
> > > domain which
> > > has been left enabled by the bootloader.
> >
> > I don't see why .probe() should depend on the runtime PM framework to call
> > .runtime_resume() for it. It looks like .probe() could just call
> > .runtime_resume() directly if needed.
> >
> > Besides, your change breaks existing code as I said.
>
> Before using gen_pd power domains we had the following flow of calls/controls:
>
> 1. fimc_probe(fimd_pdev)
> ...
> 2. pm_runtime_enable(fimd_pdev->dev)
> 3. pm_runtime_get_sync(fimd_pdev->dev)
> 3a. parent device's runtime_resume()
> 3b. fimc_runtime_resume(fimd_pdev->dev)
> ...
> 4. pm_runtime_put(fimd_pdev->dev)
> ...
> 5. (runtime put timer kicks off)
> 5a. fimc_runtime_put(fimd_pdev->dev)
> 5b. parent device's runtime_suspend()
>
> (this flow assumed that fimc device was the only child of its parent platform
> device).
>
> Now with power gen_pd driver with my patch I get the following call sequence:
>
> 1. fimc_probe(fimd_pdev)
> ...
> 2. pm_runtime_enable(fimd_pdev->dev)
> 3. pm_runtime_get_sync(fimd_pdev->dev)
> 3a. gen_pd pd_power_on(...)
> 3b. fimc_runtime_resume(fimd_pdev->dev)
> 4. pm_runtime_put(fimd_pdev->dev)
> ...
> 5. (runtime put timer kic
Re: [PATCHv2 0/3] Add support for DRM display subsystem
On 05/10/2012 11:36 AM, Kukjin Kim wrote: > Note that I have a plan to replace board files with DT supporting in > mach-exynos/ next time. What is the purpose of one side decisions like this ? I find it really annoying. We have been building those board files through multiple kernel releases, it took much effort and time to create proper drivers and extending frameworks, like video or media. It is going to take some more effort to make those subsystems DT aware and add proper support at the drivers. This is being worked on and touching existing boards doesn't sound like an incentive to me. We have to deal with multiple boards and breaking existing ones which are almost completed just adds an effort for us of maintaining them internally. The mainline kernel is going to always be a half-product, not properly tested and validated. We get new boards that need to be supported in the kernel quite frequently, those boards share device IPs, so adapting the drivers to the DT is already our goal, for getting new boards supported in the mainline. I have no idea what that "replacing" is going to be about, but if you add support for all devices present in those boards and they will all work as before - I'll happily accept that. Although I'm afraid it is just going to be one big breakage and regression, in the name of "everything must migrate to the device tree". I would really appreciate more common agreement, rather than putting a spoke in somebody else's wheel. Once all devices in current mach-exynos board files get proper DT support we are going to remove the boards ourselves. I'm sure there is lots of other things that need more attention than that. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: Add platform resource definitions for FIMC-LITE
q -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/3] Add support for DRM display subsystem
On 10 May 2012 20:16, Kyungmin Park wrote: > On Thu, May 10, 2012 at 10:48 PM, Arnd Bergmann wrote: >> On Thursday 10 May 2012, Kyungmin Park wrote: >>> > And I won't apply new feature for non-dt board file from now on. I think, >>> > we >>> > need to support DT in mach-exynos/ instead of non-DT and DT together, so >>> > please consider to move on dt supporting for Samsung mobile boards. >>> > >>> >>> Probably you misunderstand Arnd word. >>> >>> "From the statements made so far, I can see no clear policy that we can >>> apply to everyone. My take on this is that for any work I spend on >>> multiplatform kernel, I concentrate on the DT-based board files and >>> get them to work together first, but leave it up to the individual >>> subarch maintainers whether they want to add other board files into >>> the mix." >>> >>> It doesn't mean add new feature to non-DT board. don't add new board file. >> >> This is a completely separate discussion, the problem at hand doesn't >> have anything to do with building a kernel for multiple mach-* >> directories combined that I was referring to in the text you quote. >> >>> In this case, DRM is not yet ready to support DT. >>> >>> Okay, you can just drop this patches. but please note that this >>> patches are posted at Mar 13 before you decide. >> >> I think it's a good idea to make new features DT-only because this >> way we don't get any regressions and everyone who want to use >> the new feature will be able to test the DT support on his board. > The 'new feature' is some misleading word. In this case DRM drivers > are used from v3.2 at drivers. but not registered it at board file. > Basically agree to move DT and used it finally. Before that bring up > the board using DRM based graphics. >> >> This of course requires that basic DT support is available for the >> systems in question so we don't regress when moving away from the >> old board files. My impression is that we're getting close to that >> point on exynos thanks to Thomas' work on this. AFAICT we don't >> have a DT binding for screen timings yet, so you will still have >> to use auxdata for that or put the timings into the driver. > I'm also thanks to Thomas, he did a lot of works for eyxnos. but most > parts are missing at least exynos platform. > e.g., Please see the origen.dts. I wonder it's booted with any > graphical user interface platform, android or ubuntu. > Maybe not. We (Samsung Landing team) did manage to boot Origen with Ubuntu using DT support. But that is only upto home screen with minimal support. > > Anyway, as discussed in multi-platform support mail threads, we're > also moving to DT in the big picture. but still want to use existing > boards as is. For Exynos multimedia IPs, for instance, which already have drivers as well as platform support in mainline, do we still defer device registration in machine file until DT support for these IPs is added? > > Thank you, > Kyungmin Park > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- With warm regards, Sachin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/3] Add support for DRM display subsystem
On Thu, May 10, 2012 at 10:48 PM, Arnd Bergmann wrote: > On Thursday 10 May 2012, Kyungmin Park wrote: >> > And I won't apply new feature for non-dt board file from now on. I think, >> > we >> > need to support DT in mach-exynos/ instead of non-DT and DT together, so >> > please consider to move on dt supporting for Samsung mobile boards. >> > >> >> Probably you misunderstand Arnd word. >> >> "From the statements made so far, I can see no clear policy that we can >> apply to everyone. My take on this is that for any work I spend on >> multiplatform kernel, I concentrate on the DT-based board files and >> get them to work together first, but leave it up to the individual >> subarch maintainers whether they want to add other board files into >> the mix." >> >> It doesn't mean add new feature to non-DT board. don't add new board file. > > This is a completely separate discussion, the problem at hand doesn't > have anything to do with building a kernel for multiple mach-* > directories combined that I was referring to in the text you quote. > >> In this case, DRM is not yet ready to support DT. >> >> Okay, you can just drop this patches. but please note that this >> patches are posted at Mar 13 before you decide. > > I think it's a good idea to make new features DT-only because this > way we don't get any regressions and everyone who want to use > the new feature will be able to test the DT support on his board. The 'new feature' is some misleading word. In this case DRM drivers are used from v3.2 at drivers. but not registered it at board file. Basically agree to move DT and used it finally. Before that bring up the board using DRM based graphics. > > This of course requires that basic DT support is available for the > systems in question so we don't regress when moving away from the > old board files. My impression is that we're getting close to that > point on exynos thanks to Thomas' work on this. AFAICT we don't > have a DT binding for screen timings yet, so you will still have > to use auxdata for that or put the timings into the driver. I'm also thanks to Thomas, he did a lot of works for eyxnos. but most parts are missing at least exynos platform. e.g., Please see the origen.dts. I wonder it's booted with any graphical user interface platform, android or ubuntu. Maybe not. Anyway, as discussed in multi-platform support mail threads, we're also moving to DT in the big picture. but still want to use existing boards as is. Thank you, Kyungmin Park -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: SAMSUNG: Fix for S3C2412 EBI memory mapping.
On 10-05-2012 10:25, Kukjin Kim wrote:
José Miguel Gonçalves wrote:
While upgrading the kernel on a S3C2412 based board I've noted that it was
impossible to boot the board with a 2.6.32 or upper kernel.
I've tracked down the problem to the EBI virtual memory mapping that is in
conflict with the IO mapping definition in arch/arm/mach-s3c24xx/s3c2412.c.
Signed-off-by: José Miguel Gonçalves
---
arch/arm/plat-samsung/include/plat/map-s3c.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/plat-samsung/include/plat/map-s3c.h b/arch/arm/plat-
samsung/include/plat/map-s3c.h
index 7d04875..c0c70a8 100644
--- a/arch/arm/plat-samsung/include/plat/map-s3c.h
+++ b/arch/arm/plat-samsung/include/plat/map-s3c.h
@@ -22,7 +22,7 @@
#define S3C24XX_VA_WATCHDOG S3C_VA_WATCHDOG
#define S3C2412_VA_SSMC S3C_ADDR_CPU(0x)
-#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0001)
+#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0010)
#define S3C2410_PA_UART (0x5000)
#define S3C24XX_PA_UART S3C2410_PA_UART
--
1.7.5.4
Yeah, as you said, the mapping for SSMC invade EBI area but I think, just SZ_4K
is enough for SSMC. So following is better in this case. How do you think? And
there is no problem on your board?
diff --git a/arch/arm/mach-s3c24xx/s3c2412.c b/arch/arm/mach-s3c24xx/s3c2412.c
index d4bc7f9..ac906bf 100644
--- a/arch/arm/mach-s3c24xx/s3c2412.c
+++ b/arch/arm/mach-s3c24xx/s3c2412.c
@@ -72,7 +72,7 @@ static struct map_desc s3c2412_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C2412_VA_SSMC,
.pfn = __phys_to_pfn(S3C2412_PA_SSMC),
- .length = SZ_1M,
+ .length = SZ_4K,
.type= MT_DEVICE,
},
{
It does not work! I tried also a 64K length and also did not work. With your patch
my console (with earlyprintk set) only displays the following:
## Booting image at 3080 ...
Image Name: Linux-3.2.16-inov1
Created: 2012-05-10 12:42:49 UTC
Image Type: ARM Linux Kernel Image (uncompressed)
Data Size:1202592 Bytes = 1.1 MB
Load Address: 30008000
Entry Point: 30008000
Verifying Checksum ... OK
OK
Starting kernel ...
Uncompressing Linux... done, booting the kernel.
Linux version 3.2.16-inov1 (jmpg@st-ze) (gcc version 4.6.4 20120402 (prerelease)
(crosstool-NG 1.15.2) ) #3 PREEMPT Thu May 10 13:42:48 WEST 2012
CPU: ARM926EJ-S [41069265] revision 5 (ARMv5TEJ), cr=00053177
CPU: VIVT data cache, VIVT instruction cache
Machine: SMDK2412
bootconsole [earlycon0] enabled
Memory policy: ECC disabled, Data cache writeback
CPU S3C2412 (id 0x32412003)
My guess is that the MMU initialization on the S3C2412 chip only allows a minimum
of 1MB for the page size.
Best regards,
José Gonçalves
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/3] Add support for DRM display subsystem
On Thursday 10 May 2012, Kyungmin Park wrote: > > And I won't apply new feature for non-dt board file from now on. I think, > > we > > need to support DT in mach-exynos/ instead of non-DT and DT together, so > > please consider to move on dt supporting for Samsung mobile boards. > > > > Probably you misunderstand Arnd word. > > "From the statements made so far, I can see no clear policy that we can > apply to everyone. My take on this is that for any work I spend on > multiplatform kernel, I concentrate on the DT-based board files and > get them to work together first, but leave it up to the individual > subarch maintainers whether they want to add other board files into > the mix." > > It doesn't mean add new feature to non-DT board. don't add new board file. This is a completely separate discussion, the problem at hand doesn't have anything to do with building a kernel for multiple mach-* directories combined that I was referring to in the text you quote. > In this case, DRM is not yet ready to support DT. > > Okay, you can just drop this patches. but please note that this > patches are posted at Mar 13 before you decide. I think it's a good idea to make new features DT-only because this way we don't get any regressions and everyone who want to use the new feature will be able to test the DT support on his board. This of course requires that basic DT support is available for the systems in question so we don't regress when moving away from the old board files. My impression is that we're getting close to that point on exynos thanks to Thomas' work on this. AFAICT we don't have a DT binding for screen timings yet, so you will still have to use auxdata for that or put the timings into the driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] mmc: dw_mmc: lookup for optional biu and ciu clocks
On Wed, May 02, 2012 at 03:53:53PM +0100, James Hogan wrote: > Hi > > On 2 May 2012 06:07, Thomas Abraham wrote: > > + if (IS_ERR(host->biu_clk)) > > + dev_info(&host->dev, "biu clock not available\n"); > > In this case, should it set host->biu_clk to NULL or are clk_disable > and clk_put guaranteed to handle an IS_ERR value? I keep saying this. NULL must be treated as a valid reutrn value from clk_get() and must not use this as a sentinel. The clk namespace is that errors are indicated with IS_ERR(clk) returning true. Everything else must be assumed to be valid by drivers. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND 3/4] ARM: EXYNOS: Add s3c-hsotg device support for NURI board
From: Joonyoung Shim
This patch adds hsotg device to the NURI board.
Signed-off-by: Joonyoung Shim
Signed-off-by: Kyungmin Park
[Rebased on the newest git/kgene/linux-samsung #for-next]
Signed-off-by: Lukasz Majewski
---
arch/arm/mach-exynos/Kconfig |1 +
arch/arm/mach-exynos/mach-nuri.c |9 -
2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 2c35fd4..5e64728 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -287,6 +287,7 @@ config MACH_NURI
select S5P_DEV_MFC
select S5P_DEV_USB_EHCI
select S5P_SETUP_MIPIPHY
+ select S3C_DEV_USB_HSOTG
select EXYNOS4_DEV_DMA
select EXYNOS4_SETUP_FIMC
select EXYNOS4_SETUP_FIMD0
diff --git a/arch/arm/mach-exynos/mach-nuri.c b/arch/arm/mach-exynos/mach-nuri.c
index 021dc68..4402aad 100644
--- a/arch/arm/mach-exynos/mach-nuri.c
+++ b/arch/arm/mach-exynos/mach-nuri.c
@@ -348,6 +348,7 @@ static struct regulator_consumer_supply __initdata
max8997_ldo1_[] = {
REGULATOR_SUPPLY("vdd", "s5p-adc"), /* Used by CPU's ADC drv */
};
static struct regulator_consumer_supply __initdata max8997_ldo3_[] = {
+ REGULATOR_SUPPLY("vusb_d", "s3c-hsotg"), /* USB */
REGULATOR_SUPPLY("vdd11", "s5p-mipi-csis.0"), /* MIPI */
};
static struct regulator_consumer_supply __initdata max8997_ldo4_[] = {
@@ -363,7 +364,7 @@ static struct regulator_consumer_supply __initdata
max8997_ldo7_[] = {
REGULATOR_SUPPLY("dig_18", "0-001f"), /* HCD803 */
};
static struct regulator_consumer_supply __initdata max8997_ldo8_[] = {
- REGULATOR_SUPPLY("vusb_d", NULL), /* Used by CPU */
+ REGULATOR_SUPPLY("vusb_a", "s3c-hsotg"), /* USB */
REGULATOR_SUPPLY("vdac", NULL), /* Used by CPU */
};
static struct regulator_consumer_supply __initdata max8997_ldo11_[] = {
@@ -819,6 +820,7 @@ static struct regulator_init_data __initdata
max8997_esafeout1_data = {
.constraints= {
.name = "SAFEOUT1",
.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ .always_on = 1,
.state_mem = {
.disabled = 1,
},
@@ -1076,6 +1078,9 @@ static void __init nuri_ehci_init(void)
s5p_ehci_set_platdata(pdata);
}
+/* USB OTG */
+static struct s3c_hsotg_plat nuri_hsotg_pdata;
+
/* CAMERA */
static struct regulator_consumer_supply cam_vt_cam15_supply =
REGULATOR_SUPPLY("vdd_core", "6-003c");
@@ -1288,6 +1293,7 @@ static struct platform_device *nuri_devices[] __initdata
= {
&s5p_device_mfc_l,
&s5p_device_mfc_r,
&s5p_device_fimc_md,
+ &s3c_device_usb_hsotg,
/* NURI Devices */
&nuri_gpio_keys,
@@ -1336,6 +1342,7 @@ static void __init nuri_machine_init(void)
nuri_camera_init();
nuri_ehci_init();
+ s3c_hsotg_set_platdata(&nuri_hsotg_pdata);
/* Last */
platform_add_devices(nuri_devices, ARRAY_SIZE(nuri_devices));
--
1.7.2.3
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND 2/4] ARM: EXYNOS: Add s3c-hsotg device support for GONI board
This patch adds hsotg device to the GONI board.
Signed-off-by: Lukasz Majewski
Signed-off-by: Kyungmin Park
---
arch/arm/mach-s5pv210/Kconfig |1 +
arch/arm/mach-s5pv210/mach-goni.c |5 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-s5pv210/Kconfig
index 29594fc..88e983b 100644
--- a/arch/arm/mach-s5pv210/Kconfig
+++ b/arch/arm/mach-s5pv210/Kconfig
@@ -85,6 +85,7 @@ config MACH_AQUILA
select S5P_DEV_ONENAND
select S5PV210_SETUP_FB_24BPP
select S5PV210_SETUP_SDHCI
+ select S5PV210_SETUP_USB_PHY
help
Machine support for the Samsung Aquila target based on S5PC110 SoC
diff --git a/arch/arm/mach-s5pv210/mach-goni.c
b/arch/arm/mach-s5pv210/mach-goni.c
index 3239566..5fac4e4 100644
--- a/arch/arm/mach-s5pv210/mach-goni.c
+++ b/arch/arm/mach-s5pv210/mach-goni.c
@@ -278,6 +278,9 @@ static void __init goni_tsp_init(void)
i2c2_devs[0].irq = gpio_to_irq(gpio);
}
+/* USB OTG */
+static struct s3c_hsotg_plat goni_hsotg_pdata;
+
static void goni_camera_init(void)
{
s5pv210_fimc_setup_gpio(S5P_CAMPORT_A);
@@ -941,6 +944,8 @@ static void __init goni_machine_init(void)
s3c_set_platdata(&goni_fimc_md_platdata, sizeof(goni_fimc_md_platdata),
&s5p_device_fimc_md);
+ s3c_hsotg_set_platdata(&goni_hsotg_pdata);
+
goni_camera_init();
/* SPI */
--
1.7.2.3
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND 1/4] ARM: EXYNOS: Add usb otg phy control for EXYNOS4210
This patch supports to control usb otg phy of EXYNOS4210.
Signed-off-by: Joonyoung Shim
Signed-off-by: Kyungmin Park
[Rebased on the newest git/kgene/linux-samsung #for-next]
Signed-off-by: Lukasz Majewski
---
arch/arm/mach-exynos/include/mach/irqs.h |1 +
arch/arm/mach-exynos/include/mach/map.h |4 +
arch/arm/mach-exynos/include/mach/regs-pmu.h |3 +
arch/arm/mach-exynos/setup-usb-phy.c | 95 +++---
4 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/arch/arm/mach-exynos/include/mach/irqs.h
b/arch/arm/mach-exynos/include/mach/irqs.h
index 1161675..ddde8f3 100644
--- a/arch/arm/mach-exynos/include/mach/irqs.h
+++ b/arch/arm/mach-exynos/include/mach/irqs.h
@@ -196,6 +196,7 @@
#define IRQ_IIC7 EXYNOS4_IRQ_IIC7
#define IRQ_USB_HOST EXYNOS4_IRQ_USB_HOST
+#define IRQ_OTGEXYNOS4_IRQ_USB_HSOTG
#define IRQ_HSMMC0 EXYNOS4_IRQ_HSMMC0
#define IRQ_HSMMC1 EXYNOS4_IRQ_HSMMC1
diff --git a/arch/arm/mach-exynos/include/mach/map.h
b/arch/arm/mach-exynos/include/mach/map.h
index 0e2292d..2196af2 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -168,6 +168,9 @@
#define EXYNOS4_PA_HSMMC(x)(0x1251 + ((x) * 0x1))
#define EXYNOS4_PA_DWMCI 0x1255
+#define EXYNOS4_PA_HSOTG 0x1248
+#define EXYNOS4_PA_USB_HSPHY 0x125B
+
#define EXYNOS4_PA_SATA0x1256
#define EXYNOS4_PA_SATAPHY 0x125D
#define EXYNOS4_PA_SATAPHY_CTRL0x126B
@@ -224,6 +227,7 @@
#define S3C_PA_SPI0EXYNOS4_PA_SPI0
#define S3C_PA_SPI1EXYNOS4_PA_SPI1
#define S3C_PA_SPI2EXYNOS4_PA_SPI2
+#define S3C_PA_USB_HSOTG EXYNOS4_PA_HSOTG
#define S5P_PA_EHCIEXYNOS4_PA_EHCI
#define S5P_PA_FIMC0 EXYNOS4_PA_FIMC0
diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h
b/arch/arm/mach-exynos/include/mach/regs-pmu.h
index 4c53f38..d457d05 100644
--- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
+++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
@@ -163,6 +163,9 @@
#define S5P_CHECK_SLEEP0x0BAD
/* Only for EXYNOS4210 */
+#define S5P_USBDEVICE_PHY_CONTROL S5P_PMUREG(0x0704)
+#define S5P_USBDEVICE_PHY_ENABLE (1 << 0)
+
#define S5P_USBHOST_PHY_CONTROLS5P_PMUREG(0x0708)
#define S5P_USBHOST_PHY_ENABLE (1 << 0)
diff --git a/arch/arm/mach-exynos/setup-usb-phy.c
b/arch/arm/mach-exynos/setup-usb-phy.c
index 41743d2..6cf5d6a 100644
--- a/arch/arm/mach-exynos/setup-usb-phy.c
+++ b/arch/arm/mach-exynos/setup-usb-phy.c
@@ -26,11 +26,72 @@ static int exynos4_usb_host_phy_is_on(void)
return (readl(EXYNOS4_PHYPWR) & PHY1_STD_ANALOG_POWERDOWN) ? 0 : 1;
}
-static int exynos4_usb_phy1_init(struct platform_device *pdev)
+static void exynos4_usb_phy_clkset(struct platform_device *pdev)
{
- struct clk *otg_clk;
struct clk *xusbxti_clk;
u32 phyclk;
+
+ /* set clock frequency for PLL */
+ phyclk = readl(EXYNOS4_PHYCLK) & ~CLKSEL_MASK;
+
+ xusbxti_clk = clk_get(&pdev->dev, "xusbxti");
+ if (xusbxti_clk && !IS_ERR(xusbxti_clk)) {
+ switch (clk_get_rate(xusbxti_clk)) {
+ case 12 * MHZ:
+ phyclk |= CLKSEL_12M;
+ break;
+ case 24 * MHZ:
+ phyclk |= CLKSEL_24M;
+ break;
+ default:
+ case 48 * MHZ:
+ /* default reference clock */
+ break;
+ }
+ clk_put(xusbxti_clk);
+ }
+
+ writel(phyclk, EXYNOS4_PHYCLK);
+}
+
+static int exynos4_usb_phy0_init(struct platform_device *pdev)
+{
+ u32 rstcon;
+
+ writel(readl(S5P_USBDEVICE_PHY_CONTROL) | S5P_USBDEVICE_PHY_ENABLE,
+ S5P_USBDEVICE_PHY_CONTROL);
+
+ exynos4_usb_phy_clkset(pdev);
+
+ /* set to normal PHY0 */
+ writel((readl(EXYNOS4_PHYPWR) & ~PHY0_NORMAL_MASK), EXYNOS4_PHYPWR);
+
+ /* reset PHY0 and Link */
+ rstcon = readl(EXYNOS4_RSTCON) | PHY0_SWRST_MASK;
+ writel(rstcon, EXYNOS4_RSTCON);
+ udelay(10);
+
+ rstcon &= ~PHY0_SWRST_MASK;
+ writel(rstcon, EXYNOS4_RSTCON);
+ udelay(80);
+
+ return 0;
+}
+
+static int exynos4_usb_phy0_exit(struct platform_device *pdev)
+{
+ writel((readl(EXYNOS4_PHYPWR) | PHY0_ANALOG_POWERDOWN |
+ PHY0_OTG_DISABLE), EXYNOS4_PHYPWR);
+
+ writel(readl(S5P_USBDEVICE_PHY_CONTROL) & ~S5P_USBDEVICE_PHY_ENABLE,
+ S5P_USBDEVICE_PHY_CONTROL);
+
+ return 0;
+}
+
+static int exynos4_usb_phy1_init(struct platform_device *pdev
[PATCH RESEND 4/4] ARM: EXYNOS: Add s3c-hsotg device support for Universal C210 board
This patch adds platform data for using S3C-HSOTG driver at
Universal_C210 target.
Signed-off-by: Lukasz Majewski
Signed-off-by: Kyungmin Park
---
arch/arm/mach-exynos/Kconfig |2 ++
arch/arm/mach-exynos/mach-universal_c210.c | 10 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 5e64728..56d0e7e 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -246,6 +246,7 @@ config MACH_UNIVERSAL_C210
select S3C_DEV_I2C1
select S3C_DEV_I2C3
select S3C_DEV_I2C5
+ select S3C_DEV_USB_HSOTG
select S5P_DEV_I2C_HDMIPHY
select S5P_DEV_MFC
select S5P_DEV_ONENAND
@@ -259,6 +260,7 @@ config MACH_UNIVERSAL_C210
select EXYNOS4_SETUP_SDHCI
select EXYNOS4_SETUP_FIMC
select S5P_SETUP_MIPIPHY
+ select EXYNOS4_SETUP_USB_PHY
help
Machine support for Samsung Mobile Universal S5PC210 Reference
Board.
diff --git a/arch/arm/mach-exynos/mach-universal_c210.c
b/arch/arm/mach-exynos/mach-universal_c210.c
index add55b4..9781669 100644
--- a/arch/arm/mach-exynos/mach-universal_c210.c
+++ b/arch/arm/mach-exynos/mach-universal_c210.c
@@ -204,6 +204,7 @@ static struct regulator_init_data lp3974_ldo2_data = {
};
static struct regulator_consumer_supply lp3974_ldo3_consumer[] = {
+ REGULATOR_SUPPLY("vusb_a", "s3c-hsotg"),
REGULATOR_SUPPLY("vdd", "exynos4-hdmi"),
REGULATOR_SUPPLY("vdd_pll", "exynos4-hdmi"),
REGULATOR_SUPPLY("vdd11", "s5p-mipi-csis.0"),
@@ -289,6 +290,7 @@ static struct regulator_init_data lp3974_ldo7_data = {
};
static struct regulator_consumer_supply lp3974_ldo8_consumer[] = {
+ REGULATOR_SUPPLY("vusb_d", "s3c-hsotg"),
REGULATOR_SUPPLY("vdd33a_dac", "s5p-sdo"),
};
@@ -485,7 +487,10 @@ static struct regulator_init_data lp3974_vichg_data = {
static struct regulator_init_data lp3974_esafeout1_data = {
.constraints= {
.name = "SAFEOUT1",
+ .min_uV = 480,
+ .max_uV = 480,
.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ .always_on = 1,
.state_mem = {
.enabled= 1,
},
@@ -991,6 +996,9 @@ static struct gpio universal_camera_gpios[] = {
{ GPIO_CAM_VGA_NSTBY, GPIOF_OUT_INIT_LOW, "CAM_VGA_NSTBY" },
};
+/* USB OTG */
+static struct s3c_hsotg_plat universal_hsotg_pdata;
+
static void __init universal_camera_init(void)
{
s3c_set_platdata(&mipi_csis_platdata, sizeof(mipi_csis_platdata),
@@ -1046,6 +1054,7 @@ static struct platform_device *universal_devices[]
__initdata = {
&s5p_device_onenand,
&s5p_device_fimd0,
&s5p_device_jpeg,
+ &s3c_device_usb_hsotg,
&s5p_device_mfc,
&s5p_device_mfc_l,
&s5p_device_mfc_r,
@@ -1098,6 +1107,7 @@ static void __init universal_machine_init(void)
i2c_register_board_info(I2C_GPIO_BUS_12, i2c_gpio12_devs,
ARRAY_SIZE(i2c_gpio12_devs));
+ s3c_hsotg_set_platdata(&universal_hsotg_pdata);
universal_camera_init();
/* Last */
--
1.7.2.3
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND 0/4] ARM: EXYNOS: Add s3c-hsotg support for Exynos4210 and S5PV210 boards
This patch series adds S3C-HSOTG UDC support for Exynos4210 and S5PV210 based targets; namely Universal_C210, Nuri and GONI. Tested HW: - Exynos4210 NURI target rev.1 - Exynos4210 Universal_C210 target rev.0 - S5PV210 GONI target Dependencies: - usb:hsotg:samsung USB S3C-HSOTG driver fixes and code cleanup patches, which have been already pulled to usb for-next branch. http://permalink.gmane.org/gmane.linux.usb.general/62101 Those patches are enabling this UDC driver on GONI, NURI and Universal_C210 targets Joonyoung Shim (1): ARM: EXYNOS: Add s3c-hsotg device support for NURI board Lukasz Majewski (3): ARM: EXYNOS: Add usb otg phy control for EXYNOS4210 ARM: EXYNOS: Add s3c-hsotg device support for GONI board ARM: EXYNOS: Add s3c-hsotg device support for Universal C210 board arch/arm/mach-exynos/Kconfig |3 + arch/arm/mach-exynos/include/mach/irqs.h |1 + arch/arm/mach-exynos/include/mach/map.h |4 + arch/arm/mach-exynos/include/mach/regs-pmu.h |3 + arch/arm/mach-exynos/mach-nuri.c |9 ++- arch/arm/mach-exynos/mach-universal_c210.c | 10 +++ arch/arm/mach-exynos/setup-usb-phy.c | 95 +++--- arch/arm/mach-s5pv210/Kconfig|1 + arch/arm/mach-s5pv210/mach-goni.c|5 ++ 9 files changed, 105 insertions(+), 26 deletions(-) -- 1.7.2.3 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] drivers: genpd: let platform code to register devices into disabled domains
Hi Rafael,
On Monday, May 07, 2012 8:45 PM Rafael J. Wysocki wrote:
> On Monday, May 07, 2012, Marek Szyprowski wrote:
> > Hi Rafael,
> >
> > I'm sorry for a late reply, I was on holidays last week and just got back to
> > the office.
> >
> > On Sunday, April 29, 2012 10:55 PM Rafael J. Wysocki wrote:
> >
> > > On Friday, April 06, 2012, Marek Szyprowski wrote:
> > > > Some bootloaders disable power domains on boot and the platform startup
> > > > code registers them in the 'disabled' state. Current gen_pd code assumed
> > > > that the devices can be registered only to the power domain which is in
> > > > 'enabled' state and these devices are active at the time of the
> > > > registration. This is not correct in our case. This patch lets drivers
> > > > to be registered into 'disabled' power domains and finally solves
> > > > mysterious freezes and lack of resume/suspend calls on Samsung Exynos4
> > > > NURI and UniversalC210 platforms.
> > > >
> > > > Signed-off-by: Marek Szyprowski
> > > > Signed-off-by: Kyungmin Park
> > > > ---
> > > > drivers/base/power/domain.c |7 +--
> > > > 1 files changed, 1 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> > > > index 73ce9fb..05f5799f 100644
> > > > --- a/drivers/base/power/domain.c
> > > > +++ b/drivers/base/power/domain.c
> > > > @@ -1211,11 +1211,6 @@ int __pm_genpd_add_device(struct
> > > > generic_pm_domain *genpd, struct
> > > device *dev,
> > > >
> > > > genpd_acquire_lock(genpd);
> > > >
> > > > - if (genpd->status == GPD_STATE_POWER_OFF) {
> > > > - ret = -EINVAL;
> > > > - goto out;
> > > > - }
> > > > -
> > > > if (genpd->prepared_count > 0) {
> > > > ret = -EAGAIN;
> > > > goto out;
> > > > @@ -1239,7 +1234,7 @@ int __pm_genpd_add_device(struct
> > > > generic_pm_domain *genpd, struct
> > > device *dev,
> > > > dev_pm_get_subsys_data(dev);
> > > > dev->power.subsys_data->domain_data = &gpd_data->base;
> > > > gpd_data->base.dev = dev;
> > > > - gpd_data->need_restore = false;
> > > > + gpd_data->need_restore = true;
> > >
> > > I think that should be:
> > >
> > > + gpd_data->need_restore = genpd->status == GPD_STATE_POWER_OFF;
> > >
> > > Otherwise, on the next domain power off the device's state won't be saved.
> > >
> > > > list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
> > > > if (td)
> > > > gpd_data->td = *td;
> >
> > I've tested the above change and there is problem. Let me explain in detail
> > the
> > sw/hw configuration I have.
> >
> > There is a power domain and the device driver. The device itself also has
> > it's own
> > power management code (which enables and disables clocks). Some power
> > domains are
> > disabled by bootloader and some devices in the active power domains have
> > their
> > clocks disabled too. In the current runtime pm code the devices were probed
> > in
> > 'disabled' state and had to enable itself by calling get_runtime_sync(). My
> > initial
> > patch restored runtime pm handling to the old state (the same which was
> > with non
> > gen_pd based driver or no power domain driver at all, where runtime pm was
> > handled
> > by platform bus). If I apply your patch the runtime_restore
>
> I guess you mean .runtime_resume().
>
> > callback is not called on first driver probe for devices inside the domain
> > which
> > has been left enabled by the bootloader.
>
> I don't see why .probe() should depend on the runtime PM framework to call
> .runtime_resume() for it. It looks like .probe() could just call
> .runtime_resume() directly if needed.
>
> Besides, your change breaks existing code as I said.
Before using gen_pd power domains we had the following flow of calls/controls:
1. fimc_probe(fimd_pdev)
...
2. pm_runtime_enable(fimd_pdev->dev)
3. pm_runtime_get_sync(fimd_pdev->dev)
3a. parent device's runtime_resume()
3b. fimc_runtime_resume(fimd_pdev->dev)
...
4. pm_runtime_put(fimd_pdev->dev)
...
5. (runtime put timer kicks off)
5a. fimc_runtime_put(fimd_pdev->dev)
5b. parent device's runtime_suspend()
(this flow assumed that fimc device was the only child of its parent platform
device).
Now with power gen_pd driver with my patch I get the following call sequence:
1. fimc_probe(fimd_pdev)
...
2. pm_runtime_enable(fimd_pdev->dev)
3. pm_runtime_get_sync(fimd_pdev->dev)
3a. gen_pd pd_power_on(...)
3b. fimc_runtime_resume(fimd_pdev->dev)
4. pm_runtime_put(fimd_pdev->dev)
...
5. (runtime put timer kicks off)
5a. fimc_runtime_put(fimd_pdev->dev)
5b. gen_pd pd_power_off (...)
so it works like before.
Now with your suggested change I get following call sequence:
1. fimc_probe(fimc_pdev)
...
2. pm_runtime_enable(fimd_pdev->dev)
3. pm_runtime_get_sync(fimd_pdev->dev)
(gen_pd finds that t
RE: [PATCH v2] ARM: Exynos4: read initial state of power domain from hw registers
Hello,
On Thursday, May 10, 2012 11:09 AM Kukjin Kim wrote:
> Marek Szyprowski wrote:
> >
> > Some bootloaders disable unused power domains, so kernel code should
> > read the actual state from the hardware registers instead of assuming
> > that their initial state is 'on'.
> >
> > Signed-off-by: Marek Szyprowski
> > Signed-off-by: Kyungmin Park
> > ---
> > arch/arm/mach-exynos/pm_domains.c |9 ++---
> > 1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-
> > exynos/pm_domains.c
> > index 13b3068..26fc47d 100644
> > --- a/arch/arm/mach-exynos/pm_domains.c
> > +++ b/arch/arm/mach-exynos/pm_domains.c
> > @@ -151,9 +151,12 @@ static __init int exynos4_pm_init_power_domain(void)
> > if (of_have_populated_dt())
> > return exynos_pm_dt_parse_domains();
> >
> > - for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++)
> > - pm_genpd_init(&exynos4_pm_domains[idx]->pd, NULL,
> > - exynos4_pm_domains[idx]->is_off);
> > + for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++) {
> > + struct exynos_pm_domain *pd = exynos4_pm_domains[idx];
> > + int on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
> > +
> > + pm_genpd_init(&pd->pd, NULL, !on);
> > + }
> >
> > #ifdef CONFIG_S5P_DEV_FIMD0
> > exynos_pm_add_dev_to_genpd(&s5p_device_fimd0, &exynos4_pd_lcd0);
> > --
> > 1.7.1.569.g6f426
>
> As you said, if bootloader changed some hardware setting which is not
> expected value in kernel, it should be notified to kernel. But for
> pm_domaine->is_off, it should be via DT on EXYNOS now. As you know, the
> functionality has been already implemented on dt.
I really doubt that we need such complex interface to pass the information
which the driver can simply read from the register...
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCHv2 0/3] Add support for DRM display subsystem
Hello, On Thursday, May 10, 2012 11:36 AM Kukjin Kim wrote: > Marek Szyprowski wrote: > > > > Hello, > > > > This patch set adds support for Exynos DRM display subsystem for > > Universal C210 and NURI boards. Exynos DRM driver has been merged to 3.3 > > kernel tree and provides unified and more powerful alternative for > > s3c-fb and s5p-tv drivers. V2 includes update for the latest changes in > > platform data structure (added 'panel' entry). > > > > Best regards > > > > Marek Szyprowski > > Samsung Poland R&D Center > > > > Patch summary: > > > > Marek Szyprowski (3): > > ARM: Exynos: add platform device for core DRM subsystem > > ARM: Exynos: Add DRM core device support for Universal C210 board > > ARM: Exynos: Add DRM core support for NURI board > > > > arch/arm/mach-exynos/Kconfig |7 ++ > > arch/arm/mach-exynos/Makefile |1 + > > arch/arm/mach-exynos/dev-drm.c | 29 > > > arch/arm/mach-exynos/mach-nuri.c | 33 > > > > arch/arm/mach-exynos/mach-universal_c210.c | 33 > > > > arch/arm/plat-samsung/include/plat/devs.h |2 + > > 6 files changed, 105 insertions(+), 0 deletions(-) > > create mode 100644 arch/arm/mach-exynos/dev-drm.c > > > > -- > > 1.7.1.569.g6f426 > > Well, 2nd and 3rd patches can break multi-platform for EXYNOS SoCs with one > kernel image. We can switch completely to DRM driver for both boards and remove entries for s3c-fb/tv if this is really needed. IMHO it is a bit lame that the DRM driver is already in the mainline, but none of the boards use it. Even if one wants to add support for it to his own board, he cannot find any reference code for it... > And I won't apply new feature for non-dt board file from now on. I think, we > need to support DT in mach-exynos/ instead of non-DT and DT together, so > please consider to move on dt supporting for Samsung mobile boards. > > Note that I have a plan to replace board files with DT supporting in > mach-exynos/ next time. Right now the DT support is so incomplete that even basic things like gpio interrupts are not yet supported. This will be a huge regression if you remove the existing board files and replace them with DT stubs. Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: dw_mmc: add samsung exynos5250 specific extentions
On 05/10/2012 07:55 PM, Thomas Abraham wrote: > On 2 May 2012 13:19, Jaehoon Chung wrote: >> On 05/02/2012 04:01 PM, Kyungmin Park wrote: >> >>> Hi, >>> >>> On 5/2/12, Thomas Abraham wrote: The instantiation of the Synopsis Designware controller on Exynos5250 include extension for SDR and DDR specific tx/rx phase shift timing and CIU internal divider. In addition to that, the option to skip the command hold stage is also introduced. Add support for these Exynos5250 specfic extenstions. > > [...] > @@ -265,6 +266,10 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command *cmd) cmdr |= SDMMC_CMD_DAT_WR; } +if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) +if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) +cmdr |= SDMMC_USE_HOLD_REG; >>> Some other board, custom SOC also can use this HOLD register. So it's >>> not EXYNOS5250 specific one. I think we introduce the more generic >>> quirks for this instead of SOC specific. >> >> One more, I think that also need to check the IMPLEMENT_HOLD_REG bit in HCON >> register. >> It has dependency with that. > > The above code is specific to Exynos5250 and hence it is not required > to check the IMPLEMENT_HOLD_REG bit in HCON register. On Exynos5250, > the hold register is implemented and available. Right, the above code is specific for Exynos5250. But HOLD_REG should be used in other SoC. it's not only Exynos5250 specific. I want more generic code than specific code for Exynos5250. Best Regards, Jaehoon Chung > > >> As Mr.Park is mentioned, this register is clock phasing. >> In spec, card is enumerated in SDR12 or SDR25 mode, the application must >> program the use_hold_reg. > > Exynos5250 hardware manual specifies additional restrictions on the > use of hold register. The above code checks for those restrictions and > programs the USE_HOLD_REG accordingly. Please let me know if there is > any condition that is not handled by the above code. > > Thanks, > Thomas. > >> >> Best Regards, >> Jaehoon Chung >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: Exynos4: add support for GATE BLOCK clocks
Hello,
On Thursday, May 10, 2012 11:14 AM Kukjin Kim wrote:
> Marek Szyprowski wrote:
> >
> > EXYNOS4_CLKGATE_BLOCK register can be used to disable the respective
> > multimedia block hardware planes. It acts similar to the power domains.
> > This patch adds transparent support for this method of gating of
> > multimedia blocks. New clocks are added as parents to the respective
> > block bus clocks. This patch has been tested on NURI and UniversalC210
> > boards, which have bootloader which disable all multimedia blocks with
> > both GATE BLOCK method and power domains.
> >
> > Signed-off-by: Marek Szyprowski
> > Signed-off-by: Kyungmin Park
> > ---
> > arch/arm/mach-exynos/clock-exynos4.c | 54
> > ++
> > 1 files changed, 54 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos/clock-exynos4.c b/arch/arm/mach-
> > exynos/clock-exynos4.c
> > index 4287311..a2f8b2e 100644
> > --- a/arch/arm/mach-exynos/clock-exynos4.c
> > +++ b/arch/arm/mach-exynos/clock-exynos4.c
> > @@ -213,6 +213,11 @@ static int exynos4_clk_dac_ctrl(struct clk *clk, int
> > enable)
> > return s5p_gatectrl(S5P_DAC_PHY_CONTROL, clk, enable);
> > }
> >
> > +static int exynos4_clk_gate_block_ctrl(struct clk *clk, int enable)
> > +{
> > + return s5p_gatectrl(EXYNOS4_CLKGATE_BLOCK, clk, enable);
> > +}
> > +
> > /* Core list of CMU_CPU side */
> >
> > static struct clksrc_clk exynos4_clk_mout_apll = {
> > @@ -459,6 +464,37 @@ static struct clksrc_clk exynos4_clk_sclk_vpll = {
> > .reg_src = { .reg = EXYNOS4_CLKSRC_TOP0, .shift = 8, .size = 1 },
> > };
> >
> > +static struct clk exynos4_clk_gate_cam = {
> > + .name = "cam",
> > + .enable = exynos4_clk_gate_block_ctrl,
> > + .ctrlbit= (1 << 0),
> > +};
> > +
> > +static struct clk exynos4_clk_gate_tv = {
> > + .name = "tv",
> > + .enable = exynos4_clk_gate_block_ctrl,
> > + .ctrlbit= (1 << 1),
> > +};
> > +
> > +static struct clk exynos4_clk_gate_mfc = {
> > + .name = "mfc",
> > + .enable = exynos4_clk_gate_block_ctrl,
> > + .ctrlbit= (1 << 2),
> > +};
> > +
> > +static struct clk exynos4_clk_gate_lcd0 = {
> > + .name = "lcd0",
> > + .enable = exynos4_clk_gate_block_ctrl,
> > + .ctrlbit= (1 << 4),
> > +};
> > +
> > +static struct clk *exynos4_gate_clocks[] = {
> > + &exynos4_clk_gate_cam,
> > + &exynos4_clk_gate_tv,
> > + &exynos4_clk_gate_mfc,
> > + &exynos4_clk_gate_lcd0,
> > +};
> > +
> > static struct clk exynos4_init_clocks_off[] = {
> > {
> > .name = "timers",
> > @@ -470,36 +506,43 @@ static struct clk exynos4_init_clocks_off[] = {
> > .devname= "s5p-mipi-csis.0",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 4),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "csis",
> > .devname= "s5p-mipi-csis.1",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 5),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "jpeg",
> > .id = 0,
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 6),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "fimc",
> > .devname= "exynos4-fimc.0",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 0),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "fimc",
> > .devname= "exynos4-fimc.1",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 1),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "fimc",
> > .devname= "exynos4-fimc.2",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 2),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "fimc",
> > .devname= "exynos4-fimc.3",
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit= (1 << 3),
> > + .parent = &exynos4_clk_gate_cam,
> > }, {
> > .name = "hsmmc",
> > .devname= "s3c-sdhci.0",
> > @@ -534,31 +577,37 @@ static struct clk exynos4_init_clocks_off[] = {
> > .devname= "s5p-sdo",
> > .enable = exynos4_clk_ip_tv_ctrl,
> > .ctrlbit= (1 << 2),
> > + .parent = &exynos4_clk_gate_tv,
> > }, {
> > .name = "mixer",
> > .devname
Re: [PATCH 2/2] regulator: Add support for MAX77686.
Hi Mark,
On Thu, May 10, 2012 at 3:04 PM, Mark Brown
wrote:
> On Thu, May 10, 2012 at 12:54:24PM +0530, Yadwinder Singh Brar wrote:
>> On Thu, May 10, 2012 at 12:17 AM, Mark Brown
>> > On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
>
>> >> + [MAX77686_EN32KHZ_AP] = NULL,
>> >> + [MAX77686_EN32KHZ_CP] = NULL,
>
>> > Now that the generic clock API is in mainline these should be moved over
>> > to use it.
>
>> Sorry, I cann't get your point here. Please explain it little bit more.
>
> These are not regulators, these are clocks. They should use the clock
> API.
>
Ok. I got it.
>> >> + if (pdata->ramp_delay) {
>> >> + max77686->ramp_delay = pdata->ramp_delay;
>> >> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> >> + RAMP_VALUE, RAMP_MASK);
>
>> > This appears not to actually use the value passed in as platform_data.
>
>> It gets corresponding index of ramp_rate value in ramp_rate_value
>> table supported by hardware, from platform_data which we write to
>> ramp_rate control bits of control registers.
>
> Why is the driver unconditionally writing these register values here
> rather than setting the ramp delay that was passed in?
Here we are setting the max77686->ramp_delay and writing the same
value(max77686->ramp_delay << 6) at register also.
Thanks,
Yadwinder.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: dw_mmc: add samsung exynos5250 specific extentions
On 2 May 2012 23:40, Olof Johansson wrote: > Hi, > > On Tue, May 1, 2012 at 10:07 PM, Thomas Abraham > wrote: >> The instantiation of the Synopsis Designware controller on Exynos5250 >> include extension for SDR and DDR specific tx/rx phase shift timing >> and CIU internal divider. In addition to that, the option to skip the >> command hold stage is also introduced. Add support for these Exynos5250 >> specfic extenstions. >> >> Signed-off-by: Abhilash Kesavan >> Signed-off-by: Thomas Abraham >> --- >> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 33 >> +++- >> drivers/mmc/host/dw_mmc-pltfm.c | 8 + >> drivers/mmc/host/dw_mmc.c | 32 >> ++- >> drivers/mmc/host/dw_mmc.h | 13 >> include/linux/mmc/dw_mmc.h | 6 +++ >> 5 files changed, 89 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> index c1ed70e..465fc31 100644 >> --- a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt >> @@ -7,6 +7,8 @@ Required Properties: >> >> * compatible: should be one of the following >> - synopsis,dw-mshc: for controllers compliant with synopsis dw-mshc. >> + - synopsis,dw-mshc-exynos5250: for controllers with Samsung >> + Exynos5250 specific extentions. > > It makes more sense to use your own manufacturer prefix here: > > samsung,exynos5250-dw-mshc > Ok. I will modify the compatible value as you have suggested. Thanks, Thomas. > > -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: dw_mmc: add samsung exynos5250 specific extentions
On 2 May 2012 13:19, Jaehoon Chung wrote: > On 05/02/2012 04:01 PM, Kyungmin Park wrote: > >> Hi, >> >> On 5/2/12, Thomas Abraham wrote: >>> The instantiation of the Synopsis Designware controller on Exynos5250 >>> include extension for SDR and DDR specific tx/rx phase shift timing >>> and CIU internal divider. In addition to that, the option to skip the >>> command hold stage is also introduced. Add support for these Exynos5250 >>> specfic extenstions. [...] >>> @@ -265,6 +266,10 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc, >>> struct mmc_command *cmd) >>> cmdr |= SDMMC_CMD_DAT_WR; >>> } >>> >>> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) >>> + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL))) >>> + cmdr |= SDMMC_USE_HOLD_REG; >> Some other board, custom SOC also can use this HOLD register. So it's >> not EXYNOS5250 specific one. I think we introduce the more generic >> quirks for this instead of SOC specific. > > One more, I think that also need to check the IMPLEMENT_HOLD_REG bit in HCON > register. > It has dependency with that. The above code is specific to Exynos5250 and hence it is not required to check the IMPLEMENT_HOLD_REG bit in HCON register. On Exynos5250, the hold register is implemented and available. > As Mr.Park is mentioned, this register is clock phasing. > In spec, card is enumerated in SDR12 or SDR25 mode, the application must > program the use_hold_reg. Exynos5250 hardware manual specifies additional restrictions on the use of hold register. The above code checks for those restrictions and programs the USE_HOLD_REG accordingly. Please let me know if there is any condition that is not handled by the above code. Thanks, Thomas. > > Best Regards, > Jaehoon Chung > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
On 10 May 2012 16:02, James Hogan wrote:
> Hi
>
> On 2 May 2012 06:07, Thomas Abraham wrote:
>> Add device tree based discovery support.
>>
>> Signed-off-by: Thomas Abraham
>> ---
>> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 85 +
>> drivers/mmc/host/dw_mmc-pltfm.c | 24 +++
>> drivers/mmc/host/dw_mmc.c | 181
>> +++-
>> drivers/mmc/host/dw_mmc.h | 10 +
>> include/linux/mmc/dw_mmc.h | 2 +
>> 5 files changed, 296 insertions(+), 6 deletions(-)
>> create mode 100644
>> Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> new file mode 100644
>> index 000..c1ed70e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> @@ -0,0 +1,85 @@
>> +* Synopsis Designware Mobile Storage Host Controller
>> +
>> +The Synopsis designware mobile storage host controller is used to interface
>> +a SoC with storage medium such as eMMC or SD/MMC cards.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be one of the following
>> + - synopsis,dw-mshc: for controllers compliant with synopsis dw-mshc.
>
> Note that although undocumented in
> Documentation/devicetree/bindings/vendor-prefixes.txt (which it
> probably should be), the designware uart already uses a different
> prefix:
> { .compatible = "snps,dw-apb-uart" },
> http://lxr.linux.no/#linux+v3.3.5/drivers/tty/serial/8250/8250_dw.c#L165
Thanks James for pointing this out. I will change the prefix for mshc
controller bindings to be consistent with that of the uart controller.
Thanks,
Thomas.
>
> Cheers
> James
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] mmc: dw_mmc: add samsung exynos5250 specific extentions
Dear Mr. Park,
On 2 May 2012 12:31, Kyungmin Park wrote:
> Hi,
>
> On 5/2/12, Thomas Abraham wrote:
>> The instantiation of the Synopsis Designware controller on Exynos5250
>> include extension for SDR and DDR specific tx/rx phase shift timing
>> and CIU internal divider. In addition to that, the option to skip the
>> command hold stage is also introduced. Add support for these Exynos5250
>> specfic extenstions.
>>
[...]
>> +static struct dw_mci_drv_data exynos5250_drv_data = {
>> + .ctrl_type = DW_MCI_TYPE_EXYNOS5250,
>> + .caps = MMC_CAP_UHS_DDR50 | MMC_CAP_1_8V_DDR |
>> + MMC_CAP_8_BIT_DATA | MMC_CAP_CMD23,
> These caps should be board specific. So it's not proper place. Esp.,
> MMC_CAP_8_BIT_DATA.
Yes, this is incorrect. I will fix this. Thanks for pointing this out.
>> +};
>> +
>> static const struct of_device_id dw_mci_pltfm_match[] = {
>> { .compatible = "synopsis,dw-mshc",
>> .data = (void *)&synopsis_drv_data, },
>> + { .compatible = "synopsis,dw-mshc-exynos5250",
>> + .data = (void *)&exynos5250_drv_data, },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, dw_mci_pltfm_match);
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index bcf66d7..9174a69 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -236,6 +236,7 @@ static void dw_mci_set_timeout(struct dw_mci *host)
>> static u32 dw_mci_prepare_command(struct mmc_host *mmc, struct mmc_command
>> *cmd)
>> {
>> struct mmc_data *data;
>> + struct dw_mci_slot *slot = mmc_priv(mmc);
>> u32 cmdr;
>> cmd->error = -EINPROGRESS;
>>
>> @@ -265,6 +266,10 @@ static u32 dw_mci_prepare_command(struct mmc_host *mmc,
>> struct mmc_command *cmd)
>> cmdr |= SDMMC_CMD_DAT_WR;
>> }
>>
>> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250)
>> + if (SDMMC_CLKSEL_GET_SELCLK_DRV(mci_readl(slot->host, CLKSEL)))
>> + cmdr |= SDMMC_USE_HOLD_REG;
> Some other board, custom SOC also can use this HOLD register. So it's
> not EXYNOS5250 specific one. I think we introduce the more generic
> quirks for this instead of SOC specific.
The above code is Exynos5250 specific. The Exynos5250 hardware manual
specifies additional restrictions on when the hold register can be
used. These restrictions are specific to Exynos5250 and hence there is
check for type of the controller.
>> +
>> return cmdr;
>> }
>>
>> @@ -787,10 +792,19 @@ static void dw_mci_set_ios(struct mmc_host *mmc,
>> struct mmc_ios *ios)
>> regs = mci_readl(slot->host, UHS_REG);
>>
>> /* DDR mode set */
>> - if (ios->timing == MMC_TIMING_UHS_DDR50)
>> + if (ios->timing == MMC_TIMING_UHS_DDR50) {
>> regs |= (0x1 << slot->id) << 16;
>> - else
>> + mci_writel(slot->host, CLKSEL, slot->host->ddr_timing);
> As you wrote, does CLKSEL is some SOC specific registers?
Yes, the CLKSEL is a Exynos specific register.
>> + } else {
>> regs &= ~(0x1 << slot->id) << 16;
>> + mci_writel(slot->host, CLKSEL, slot->host->sdr_timing);
>> + }
>> +
>> + if (slot->host->drv_data->ctrl_type == DW_MCI_TYPE_EXYNOS5250) {
>> + slot->host->bus_hz = clk_get_rate(slot->host->ciu_clk);
>> + slot->host->bus_hz /= SDMMC_CLKSEL_GET_DIVRATIO(
>> + mci_readl(slot->host, CLKSEL));
>> + }
>>
>> mci_writel(slot->host, UHS_REG, regs);
>>
>> @@ -2074,6 +2088,20 @@ static struct dw_mci_board *dw_mci_parse_dt(struct
>> dw_mci *host)
>> if (of_get_property(np, of_quriks[idx].quirk, NULL))
>> pdata->quirks |= of_quriks[idx].id;
>>
>> + if (of_property_read_u32_array(dev->of_node,
>> + "samsung,dw-mshc-sdr-timing", timing, 3))
>> + host->sdr_timing = DW_MCI_DEF_SDR_TIMING;
>> + else
>> + host->sdr_timing = SDMMC_CLKSEL_TIMING(timing[0],
>> + timing[1], timing[2]);
>> +
>> + if (of_property_read_u32_array(dev->of_node,
>> + "samsung,dw-mshc-ddr-timing", timing, 3))
>> + host->ddr_timing = DW_MCI_DEF_DDR_TIMING;
>> + else
>> + host->ddr_timing = SDMMC_CLKSEL_TIMING(timing[0],
>> + timing[1], timing[2]);
>> +
>> if (of_property_read_u32(np, "fifo-depth", &pdata->fifo_depth))
>> dev_info(dev, "fifo-depth property not found, using "
>> "value of FIFOTH register as default\n");
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 8b8862b..4b7e42b 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -53,6 +53,7 @@
>> #define SDMMC_IDINTEN 0x090
>> #define SDMMC_DSCADDR 0x094
>> #define SDMMC_BUFADDR
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
Hi
On 2 May 2012 06:07, Thomas Abraham wrote:
> Add device tree based discovery support.
>
> Signed-off-by: Thomas Abraham
> ---
> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 85 +
> drivers/mmc/host/dw_mmc-pltfm.c | 24 +++
> drivers/mmc/host/dw_mmc.c | 181
> +++-
> drivers/mmc/host/dw_mmc.h | 10 +
> include/linux/mmc/dw_mmc.h | 2 +
> 5 files changed, 296 insertions(+), 6 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
> new file mode 100644
> index 000..c1ed70e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
> @@ -0,0 +1,85 @@
> +* Synopsis Designware Mobile Storage Host Controller
> +
> +The Synopsis designware mobile storage host controller is used to interface
> +a SoC with storage medium such as eMMC or SD/MMC cards.
> +
> +Required Properties:
> +
> +* compatible: should be one of the following
> + - synopsis,dw-mshc: for controllers compliant with synopsis dw-mshc.
Note that although undocumented in
Documentation/devicetree/bindings/vendor-prefixes.txt (which it
probably should be), the designware uart already uses a different
prefix:
{ .compatible = "snps,dw-apb-uart" },
http://lxr.linux.no/#linux+v3.3.5/drivers/tty/serial/8250/8250_dw.c#L165
Cheers
James
> +
> +* reg: physical base address of the dw-mshc controller and size of its memory
> + region.
> +
> +* interrupts: interrupt specifier for the controller. The format and value of
> + the interrupt specifier depends on the interrupt parent for the controller.
> +
> +# Slots: The slot specific information are contained within child-nodes with
> + each child-node representing a supported slot. There should be atleast one
> + child node representing a card slot. The name of the slot child node should
> + be 'slot{n}' where n is the unique number of the slot connnected to the
> + controller. The following are optional properties which can be included in
> + the slot child node.
> +
> + * bus-width: specifies the width of the data bus connected from the
> + controller to the card slot. The value should be 1, 4 or 8. In case
> + this property is not specified, a default value of 1 is assumed for
> + this property.
> +
> + * cd-gpios: specifies the card detect gpio line. The format of the
> + gpio specifier depends on the gpio controller.
> +
> + * wp-gpios: specifies the write protect gpio line. The format of the
> + gpio specifier depends on the gpio controller.
> +
> + * gpios: specifies a list of gpios used for command, clock and data
> + bus. The first gpio is the command line and the second gpio is the
> + clock line. The rest of the gpios (depending on the bus-width
> + property) are the data lines in no particular order. The format of
> + the gpio specifier depends on the gpio controller.
> +
> +Optional properties:
> +
> +* fifo-depth: The maximum size of the tx/rx fifo's. If this property is not
> + specified, the default value of the fifo size is determined from the
> + controller registers.
> +
> +* card-detect-delay: Delay in milli-seconds before detecting card after card
> + insert event. The default value is 0.
> +
> +* supports-highspeed: Enables support for high speed cards (upto 50MHz)
> +
> +* card-detection-broken: The card detection functionality is not available on
> + any of the slots.
> +
> +* no-write-protect: The write protect pad of the controller is not connected
> + to the write protect pin on the slot.
> +
> +Example:
> +
> + The MSHC controller node can be split into two portions, SoC specific and
> + board specific portions as listed below.
> +
> + dwmmc0@1220 {
> + compatible = "synopsis,dw-mshc";
> + reg = <0x1220 0x1000>;
> + interrupts = <0 75 0>;
> + };
> +
> + dwmmc0@1220 {
> + supports-highspeed;
> + card-detection-broken;
> + no-write-protect;
> + fifo-depth = <0x80>;
> + card-detect-delay = <200>;
> +
> + slot0 {
> + bus-width = <8>;
> + cd-gpios = <&gpc0 2 2 3 3>;
> + gpios = <&gpc0 0 2 0 3>, <&gpc0 1 2 0 3>,
> + <&gpc1 0 2 3 3>, <&gpc1 1 2 3 3>,
> + <&gpc1 2 2 3 3>, <&gpc1 3 2 3 3>,
> + <&gpc0 3 2 3 3>, <&gpc0 4 2 3 3>,
> + <&gpc0 5 2 3 3>, <&gpc0 6 2 3 3>;
> + };
> + };
> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
> index 92ec3eb..2b2c9b
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
Hi Olof,
On 2 May 2012 23:37, Olof Johansson wrote:
> Hi,
[...]
>> +# Slots: The slot specific information are contained within child-nodes with
>> + each child-node representing a supported slot. There should be atleast one
>> + child node representing a card slot. The name of the slot child node
>> should
>> + be 'slot{n}' where n is the unique number of the slot connnected to the
>> + controller. The following are optional properties which can be included in
>> + the slot child node.
>
> Since we're talking slots / cards on a bus, I think the addressing
> model would be useful here. So in the main controller node:
> #address-cells = <1>;
> #size-cells = <0>;
>
> And then each slot would need a reg property and possibly unit address:
>
> slot {
> reg = <0>;
> ...
> };
>
> (unit addresses on the slots are only needed if they can't be
> disambiguated by name, so not needed if you only have one slot).
>
Is the addressing model as described above needed in this case? The
address for a slot is not used by the controller driver code and is
just a virtual number. It would be sufficient to represent the nodes
representing the slots with a unique name.
Thanks,
Thomas.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
On 10 May 2012 15:29, Kyungmin Park wrote: > On 5/10/12, Thomas Abraham wrote: >> Dear Mr. Park, >> >> On 2 May 2012 12:25, Kyungmin Park wrote: >>> Hi, [...] >>> I googled the "Synopsis Designware Mobile Storage Host Controller" and >>> Synopsis released it as this name. but still I like the 'dw-mmc' >>> instead of'dw-mshc'. >> >> Ok. Synopsis abbreviates this controller as MSHC in their datasheets >> available online. Since device tree is more about describing the >> hardware, using MSHC as the abbreviation will help with correlating >> hardware specs with the dts file. So I would prefer to continue using >> mshc as the abbreviation. > > Then why author of this file uses "dw-mmc" instead of "dw-mshc"? and > it uses 'dw_mci' prefix at functions. I just worried and don't want to > give confusion to users who uses this device. The device tree source files are independent of any OS specific implementations. The linux driver for this controller uses dw-mmc and dw_mci but drivers of this controller for other operating systems could choose use other names. Since Synopsis calls this controller as mshc, the dts file would be closer to hardware specs if 'mshc' is used to describe the controller. [...] +/* Variations in the dw_mci controller */ +#define DW_MCI_TYPE_SYNOPSIS 0 +#define DW_MCI_TYPE_EXYNOS5250 1 /* Samsung Exynos5250 Extensions */ >>> Um. it's not good idea to add specific SOC version. And as you know, >>> exynos4 series has this controller. >> >> There are some differences between the Exynos4 and Exynos5 mshc >> controllers. For instance, the DIVRATIO field in the CLKSEL register >> is available only in Exynos5 and there are 8 phase shift values in >> Exynos5 when compared to 4 phase shift values in Exynos4. Likewise, >> there are some other differences. So to handle these specific >> implementations, we need to define types (or variants) of the >> controller. Using SoC names for the type would help in readability of >> the different types of implementations that are defined. So I would >> prefer to continue using SoC names for this. > > You mean if it supports the exynos4, then it should be add exynos4 type? > If the existing driver requires any Exynos4 specific extensions to operate the controller correctly on Exynos4, then yes we should add a Exynos4 type. The Exynos4 and Exynos5 implementations of this controller vary in certain aspects and so we might need a Exynos4 type as well. Thanks, Thomas. [...] -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: EXYNOS: Add platform resource definitions for FIMC-LITE
On 05/10/2012 11:56 AM, Kukjin Kim wrote: >> Should I resend the patch or could you remove " and Exynos5 SoCs" >> from the description ? >> > I see, let me fix it when I apply this. > > Thanks. > Alright, thanks a lot. -- Sylwester Nawrocki Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
On 5/10/12, Thomas Abraham wrote:
> Dear Mr. Park,
>
> On 2 May 2012 12:25, Kyungmin Park wrote:
>> Hi,
>>
>> On 5/2/12, Thomas Abraham wrote:
>>> Add device tree based discovery support.
>>>
>>> Signed-off-by: Thomas Abraham
>>> ---
>>> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 85 +
>>> drivers/mmc/host/dw_mmc-pltfm.c| 24 +++
>>> drivers/mmc/host/dw_mmc.c | 181
>>> +++-
>>> drivers/mmc/host/dw_mmc.h | 10 +
>>> include/linux/mmc/dw_mmc.h |2 +
>>> 5 files changed, 296 insertions(+), 6 deletions(-)
>>> create mode 100644
>>> Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>> new file mode 100644
>>> index 000..c1ed70e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>> @@ -0,0 +1,85 @@
>>> +* Synopsis Designware Mobile Storage Host Controller
>>> +
>>> +The Synopsis designware mobile storage host controller is used to
>>> interface
>>> +a SoC with storage medium such as eMMC or SD/MMC cards.
>>> +
>>> +Required Properties:
>>> +
>>> +* compatible: should be one of the following
>>> + - synopsis,dw-mshc: for controllers compliant with synopsis
>>> dw-mshc.
>> I googled the "Synopsis Designware Mobile Storage Host Controller" and
>> Synopsis released it as this name. but still I like the 'dw-mmc'
>> instead of'dw-mshc'.
>
> Ok. Synopsis abbreviates this controller as MSHC in their datasheets
> available online. Since device tree is more about describing the
> hardware, using MSHC as the abbreviation will help with correlating
> hardware specs with the dts file. So I would prefer to continue using
> mshc as the abbreviation.
Then why author of this file uses "dw-mmc" instead of "dw-mshc"? and
it uses 'dw_mci' prefix at functions. I just worried and don't want to
give confusion to users who uses this device.
>
>>> +
>>> +* reg: physical base address of the dw-mshc controller and size of its
>>> memory
>>> + region.
>>> +
>>> +* interrupts: interrupt specifier for the controller. The format and
>>> value
>>> of
>>> + the interrupt specifier depends on the interrupt parent for the
>>> controller.
>>> +
>>> +# Slots: The slot specific information are contained within child-nodes
>>> with
>>> + each child-node representing a supported slot. There should be
>>> atleast
>>> one
>>> + child node representing a card slot. The name of the slot child node
>>> should
>>> + be 'slot{n}' where n is the unique number of the slot connnected to
>>> the
>>> + controller. The following are optional properties which can be
>>> included
>>> in
>>> + the slot child node.
>>> +
>>> + * bus-width: specifies the width of the data bus connected from
>>> the
>>> + controller to the card slot. The value should be 1, 4 or 8. In
>>> case
>>> + this property is not specified, a default value of 1 is assumed
>>> for
>>> + this property.
>>> +
>>> + * cd-gpios: specifies the card detect gpio line. The format of the
>>> + gpio specifier depends on the gpio controller.
>>> +
>>> + * wp-gpios: specifies the write protect gpio line. The format of
>>> the
>>> + gpio specifier depends on the gpio controller.
>>> +
>>> + * gpios: specifies a list of gpios used for command, clock and
>>> data
>>> + bus. The first gpio is the command line and the second gpio is
>>> the
>>> + clock line. The rest of the gpios (depending on the bus-width
>>> + property) are the data lines in no particular order. The format
>>> of
>>> + the gpio specifier depends on the gpio controller.
>>> +
>>> +Optional properties:
>>> +
>>> +* fifo-depth: The maximum size of the tx/rx fifo's. If this property is
>>> not
>>> + specified, the default value of the fifo size is determined from the
>>> + controller registers.
>>> +
>>> +* card-detect-delay: Delay in milli-seconds before detecting card
>>> after
>>> card
>>> + insert event. The default value is 0.
>>> +
>>> +* supports-highspeed: Enables support for high speed cards (upto 50MHz)
>>> +
>>> +* card-detection-broken: The card detection functionality is not
>>> available
>>> on
>>> + any of the slots.
>>> +
>>> +* no-write-protect: The write protect pad of the controller is not
>>> connected
>>> + to the write protect pin on the slot.
>>> +
>>> +Example:
>>> +
>>> + The MSHC controller node can be split into two portions, SoC specific
>>> and
>>> + board specific portions as listed below.
>>> +
>>> + dwmmc0@1220 {
>>> + compatible = "synopsis,dw-mshc";
>>> + reg = <0x1220 0x1000>;
>>> + interrupts = <0 75 0>;
>>> + };
>>> +
>>> + dwmmc0@1220 {
>>> + supports-highspeed;
>>> + card-detection-broken;
>>> +
RE: [PATCH] ARM: EXYNOS: Add platform resource definitions for FIMC-LITE
Sylwester Nawrocki wrote:
>
> On 05/08/2012 07:42 AM, Kukjin Kim wrote:
> > Sylwester Nawrocki wrote:
> >>
> >> Add the gate clocks and register region address definition for
> >> FIMC-LITE devices available in Exynos4x12 and Exynos5 SoCs.
> >>
> > This is right description? I can't find your changes for EXYNOS4412 and
> > EXYNOS5 here. Only this is for EXYNOS4212 SoC.
>
> Sorry, I wasn't precise enough. EXYNOS4_PA_FIMC_LITE is for EXYNOS4212
> and EXYNOS4412 though. Please note it is initially needed for DT
platforms,
> until we get proper clock DT bindings for EXYNOS. Is there, BTW, anyone
> known to work on converting EXYNOS to the common clock framework ?
>
Yes, my colleague is working on common clock for all of exynos stuff.
> This patch was intended for EXYNOS4212/4412. AFAICS clock code in
> mach-exynos/clock-exynos4412.c is a subset of the EXYNOS4412 platform:
>
OK.
> static void __init exynos4_init_clocks(int xtal)
> {
> ...
> if (soc_is_exynos4210())
> exynos4210_register_clocks();
> else if (soc_is_exynos4212() || soc_is_exynos4412())
> exynos4212_register_clocks();
>
> exynos4_register_clocks();
> ...
> }
>
> Should I resend the patch or could you remove " and Exynos5 SoCs"
> from the description ?
>
I see, let me fix it when I apply this.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 0/3] Add support for DRM display subsystem
On 5/10/12, Kukjin Kim wrote: > Marek Szyprowski wrote: >> >> Hello, >> >> This patch set adds support for Exynos DRM display subsystem for >> Universal C210 and NURI boards. Exynos DRM driver has been merged to 3.3 >> kernel tree and provides unified and more powerful alternative for >> s3c-fb and s5p-tv drivers. V2 includes update for the latest changes in >> platform data structure (added 'panel' entry). >> >> Best regards >> >> Marek Szyprowski >> Samsung Poland R&D Center >> >> Patch summary: >> >> Marek Szyprowski (3): >> ARM: Exynos: add platform device for core DRM subsystem >> ARM: Exynos: Add DRM core device support for Universal C210 board >> ARM: Exynos: Add DRM core support for NURI board >> >> arch/arm/mach-exynos/Kconfig |7 ++ >> arch/arm/mach-exynos/Makefile |1 + >> arch/arm/mach-exynos/dev-drm.c | 29 > >> arch/arm/mach-exynos/mach-nuri.c | 33 >> >> arch/arm/mach-exynos/mach-universal_c210.c | 33 >> >> arch/arm/plat-samsung/include/plat/devs.h |2 + >> 6 files changed, 105 insertions(+), 0 deletions(-) >> create mode 100644 arch/arm/mach-exynos/dev-drm.c >> >> -- >> 1.7.1.569.g6f426 > > Well, 2nd and 3rd patches can break multi-platform for EXYNOS SoCs with one > kernel image. > > And I won't apply new feature for non-dt board file from now on. I think, > we > need to support DT in mach-exynos/ instead of non-DT and DT together, so > please consider to move on dt supporting for Samsung mobile boards. > Probably you misunderstand Arnd word. "From the statements made so far, I can see no clear policy that we can apply to everyone. My take on this is that for any work I spend on multiplatform kernel, I concentrate on the DT-based board files and get them to work together first, but leave it up to the individual subarch maintainers whether they want to add other board files into the mix." It doesn't mean add new feature to non-DT board. don't add new board file. In this case, DRM is not yet ready to support DT. Okay, you can just drop this patches. but please note that this patches are posted at Mar 13 before you decide. BR, Kyungmin Park > Note that I have a plan to replace board files with DT supporting in > mach-exynos/ next time. > > Thanks. > > Best regards, > Kgene. > -- > Kukjin Kim , Senior Engineer, > SW Solution Development Team, Samsung Electronics Co., Ltd. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] mmc: dw_mmc: add device tree support
Dear Mr. Park,
On 2 May 2012 12:25, Kyungmin Park wrote:
> Hi,
>
> On 5/2/12, Thomas Abraham wrote:
>> Add device tree based discovery support.
>>
>> Signed-off-by: Thomas Abraham
>> ---
>> .../devicetree/bindings/mmc/synposis-dw-mshc.txt | 85 +
>> drivers/mmc/host/dw_mmc-pltfm.c | 24 +++
>> drivers/mmc/host/dw_mmc.c | 181
>> +++-
>> drivers/mmc/host/dw_mmc.h | 10 +
>> include/linux/mmc/dw_mmc.h | 2 +
>> 5 files changed, 296 insertions(+), 6 deletions(-)
>> create mode 100644
>> Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> new file mode 100644
>> index 000..c1ed70e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/synposis-dw-mshc.txt
>> @@ -0,0 +1,85 @@
>> +* Synopsis Designware Mobile Storage Host Controller
>> +
>> +The Synopsis designware mobile storage host controller is used to
>> interface
>> +a SoC with storage medium such as eMMC or SD/MMC cards.
>> +
>> +Required Properties:
>> +
>> +* compatible: should be one of the following
>> + - synopsis,dw-mshc: for controllers compliant with synopsis dw-mshc.
> I googled the "Synopsis Designware Mobile Storage Host Controller" and
> Synopsis released it as this name. but still I like the 'dw-mmc'
> instead of'dw-mshc'.
Ok. Synopsis abbreviates this controller as MSHC in their datasheets
available online. Since device tree is more about describing the
hardware, using MSHC as the abbreviation will help with correlating
hardware specs with the dts file. So I would prefer to continue using
mshc as the abbreviation.
>> +
>> +* reg: physical base address of the dw-mshc controller and size of its
>> memory
>> + region.
>> +
>> +* interrupts: interrupt specifier for the controller. The format and value
>> of
>> + the interrupt specifier depends on the interrupt parent for the
>> controller.
>> +
>> +# Slots: The slot specific information are contained within child-nodes
>> with
>> + each child-node representing a supported slot. There should be atleast
>> one
>> + child node representing a card slot. The name of the slot child node
>> should
>> + be 'slot{n}' where n is the unique number of the slot connnected to the
>> + controller. The following are optional properties which can be included
>> in
>> + the slot child node.
>> +
>> + * bus-width: specifies the width of the data bus connected from the
>> + controller to the card slot. The value should be 1, 4 or 8. In case
>> + this property is not specified, a default value of 1 is assumed for
>> + this property.
>> +
>> + * cd-gpios: specifies the card detect gpio line. The format of the
>> + gpio specifier depends on the gpio controller.
>> +
>> + * wp-gpios: specifies the write protect gpio line. The format of the
>> + gpio specifier depends on the gpio controller.
>> +
>> + * gpios: specifies a list of gpios used for command, clock and data
>> + bus. The first gpio is the command line and the second gpio is the
>> + clock line. The rest of the gpios (depending on the bus-width
>> + property) are the data lines in no particular order. The format of
>> + the gpio specifier depends on the gpio controller.
>> +
>> +Optional properties:
>> +
>> +* fifo-depth: The maximum size of the tx/rx fifo's. If this property is
>> not
>> + specified, the default value of the fifo size is determined from the
>> + controller registers.
>> +
>> +* card-detect-delay: Delay in milli-seconds before detecting card after
>> card
>> + insert event. The default value is 0.
>> +
>> +* supports-highspeed: Enables support for high speed cards (upto 50MHz)
>> +
>> +* card-detection-broken: The card detection functionality is not available
>> on
>> + any of the slots.
>> +
>> +* no-write-protect: The write protect pad of the controller is not
>> connected
>> + to the write protect pin on the slot.
>> +
>> +Example:
>> +
>> + The MSHC controller node can be split into two portions, SoC specific
>> and
>> + board specific portions as listed below.
>> +
>> + dwmmc0@1220 {
>> + compatible = "synopsis,dw-mshc";
>> + reg = <0x1220 0x1000>;
>> + interrupts = <0 75 0>;
>> + };
>> +
>> + dwmmc0@1220 {
>> + supports-highspeed;
>> + card-detection-broken;
>> + no-write-protect;
>> + fifo-depth = <0x80>;
>> + card-detect-delay = <200>;
>> +
>> + slot0 {
>> + bus-width = <8>;
>> + cd-gpios = <&gpc0 2 2 3 3>;
>> + gpios = <&gpc0 0 2 0 3>, <&gpc0 1 2 0 3>,
>> + <&gpc1 0 2 3 3>, <&gpc1 1 2 3 3>,
>> + <&gpc1 2 2
RE: [PATCHv2 0/3] Add support for DRM display subsystem
Marek Szyprowski wrote: > > Hello, > > This patch set adds support for Exynos DRM display subsystem for > Universal C210 and NURI boards. Exynos DRM driver has been merged to 3.3 > kernel tree and provides unified and more powerful alternative for > s3c-fb and s5p-tv drivers. V2 includes update for the latest changes in > platform data structure (added 'panel' entry). > > Best regards > > Marek Szyprowski > Samsung Poland R&D Center > > Patch summary: > > Marek Szyprowski (3): > ARM: Exynos: add platform device for core DRM subsystem > ARM: Exynos: Add DRM core device support for Universal C210 board > ARM: Exynos: Add DRM core support for NURI board > > arch/arm/mach-exynos/Kconfig |7 ++ > arch/arm/mach-exynos/Makefile |1 + > arch/arm/mach-exynos/dev-drm.c | 29 > arch/arm/mach-exynos/mach-nuri.c | 33 > > arch/arm/mach-exynos/mach-universal_c210.c | 33 > > arch/arm/plat-samsung/include/plat/devs.h |2 + > 6 files changed, 105 insertions(+), 0 deletions(-) > create mode 100644 arch/arm/mach-exynos/dev-drm.c > > -- > 1.7.1.569.g6f426 Well, 2nd and 3rd patches can break multi-platform for EXYNOS SoCs with one kernel image. And I won't apply new feature for non-dt board file from now on. I think, we need to support DT in mach-exynos/ instead of non-DT and DT together, so please consider to move on dt supporting for Samsung mobile boards. Note that I have a plan to replace board files with DT supporting in mach-exynos/ next time. Thanks. Best regards, Kgene. -- Kukjin Kim , Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] regulator: Add support for MAX77686.
On Thu, May 10, 2012 at 12:54:24PM +0530, Yadwinder Singh Brar wrote:
> On Thu, May 10, 2012 at 12:17 AM, Mark Brown
> > On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
> >> + [MAX77686_EN32KHZ_AP] = NULL,
> >> + [MAX77686_EN32KHZ_CP] = NULL,
> > Now that the generic clock API is in mainline these should be moved over
> > to use it.
> Sorry, I cann't get your point here. Please explain it little bit more.
These are not regulators, these are clocks. They should use the clock
API.
> >> + if (pdata->ramp_delay) {
> >> + max77686->ramp_delay = pdata->ramp_delay;
> >> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
> >> + RAMP_VALUE, RAMP_MASK);
> > This appears not to actually use the value passed in as platform_data.
> It gets corresponding index of ramp_rate value in ramp_rate_value
> table supported by hardware, from platform_data which we write to
> ramp_rate control bits of control registers.
Why is the driver unconditionally writing these register values here
rather than setting the ramp delay that was passed in?
signature.asc
Description: Digital signature
RE: [PATCH] ARM: SAMSUNG: Fix for S3C2412 EBI memory mapping.
José Miguel Gonçalves wrote:
>
> While upgrading the kernel on a S3C2412 based board I've noted that it was
> impossible to boot the board with a 2.6.32 or upper kernel.
> I've tracked down the problem to the EBI virtual memory mapping that is in
> conflict with the IO mapping definition in arch/arm/mach-s3c24xx/s3c2412.c.
>
> Signed-off-by: José Miguel Gonçalves
> ---
> arch/arm/plat-samsung/include/plat/map-s3c.h |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/include/plat/map-s3c.h b/arch/arm/plat-
> samsung/include/plat/map-s3c.h
> index 7d04875..c0c70a8 100644
> --- a/arch/arm/plat-samsung/include/plat/map-s3c.h
> +++ b/arch/arm/plat-samsung/include/plat/map-s3c.h
> @@ -22,7 +22,7 @@
> #define S3C24XX_VA_WATCHDOG S3C_VA_WATCHDOG
>
> #define S3C2412_VA_SSMC S3C_ADDR_CPU(0x)
> -#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0001)
> +#define S3C2412_VA_EBI S3C_ADDR_CPU(0x0010)
>
> #define S3C2410_PA_UART (0x5000)
> #define S3C24XX_PA_UART S3C2410_PA_UART
> --
> 1.7.5.4
Yeah, as you said, the mapping for SSMC invade EBI area but I think, just SZ_4K
is enough for SSMC. So following is better in this case. How do you think? And
there is no problem on your board?
diff --git a/arch/arm/mach-s3c24xx/s3c2412.c b/arch/arm/mach-s3c24xx/s3c2412.c
index d4bc7f9..ac906bf 100644
--- a/arch/arm/mach-s3c24xx/s3c2412.c
+++ b/arch/arm/mach-s3c24xx/s3c2412.c
@@ -72,7 +72,7 @@ static struct map_desc s3c2412_iodesc[] __initdata = {
{
.virtual = (unsigned long)S3C2412_VA_SSMC,
.pfn = __phys_to_pfn(S3C2412_PA_SSMC),
- .length = SZ_1M,
+ .length = SZ_4K,
.type= MT_DEVICE,
},
{
Thanks.
Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] mmc: dw_mmc: lookup for optional biu and ciu clocks
On 2 May 2012 20:23, James Hogan wrote:
> Hi
>
> On 2 May 2012 06:07, Thomas Abraham wrote:
>> Some platforms allow for clock gating and control of bus interface unit clock
>> and card interface unit clock. Add support for clock lookup of optional biu
>> and ciu clocks for clock gating and clock speed determination.
>>
>> Signed-off-by: Abhilash Kesavan
>> Signed-off-by: Thomas Abraham
>> ---
>> drivers/mmc/host/dw_mmc.c | 35 +++
>> include/linux/mmc/dw_mmc.h | 4
>> 2 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 1532357..036846f 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1938,19 +1938,35 @@ int dw_mci_probe(struct dw_mci *host)
>> return -ENODEV;
>> }
>>
>> - if (!host->pdata->bus_hz) {
>> + host->biu_clk = clk_get(&host->dev, "biu");
>
> These clock names sound quite platform specific (what if they're
> called something else on another platform, or another platform has
> separate ones for different instantiations of the block?). Perhaps the
> clk names should get passed in through platform data. I haven't looked
> how other drivers handle that though.
The clock names 'biu' and 'ciu' are derived from the terminology used
by the data sheet of the mshc controller. The 'biu' clock is the
source clock for the bus interface unit and 'ciu' clock is the clock
source for card interface unit of the controller. So these names are
generic and not specific to a platform. Passing clock names from
platform data is generally frowned upon.
>
>> + if (IS_ERR(host->biu_clk))
>> + dev_info(&host->dev, "biu clock not available\n");
>
> In this case, should it set host->biu_clk to NULL or are clk_disable
> and clk_put guaranteed to handle an IS_ERR value?
Yes, the clk_disable will have to be preceded with a IS_ERR check. I
will fix this.
>
>> + else
>> + clk_enable(host->biu_clk);
>> +
>> + host->ciu_clk = clk_get(&host->dev, "ciu");
>> + if (IS_ERR(host->ciu_clk))
>> + dev_info(&host->dev, "ciu clock not available\n");
>
> same here
Ok, this also will be fixed.
>
>> + else
>> + clk_enable(host->ciu_clk);
>> +
>> + if (IS_ERR(host->ciu_clk))
>> + host->bus_hz = host->pdata->bus_hz;
>> + else
>> + host->bus_hz = clk_get_rate(host->ciu_clk);
>> +
>> + if (!host->bus_hz) {
>> dev_err(&host->dev,
>> "Platform data must supply bus speed\n");
>> - return -ENODEV;
>> + ret = -ENODEV;
>> + goto err_clk;
>> }
>>
>> - host->bus_hz = host->pdata->bus_hz;
>> host->quirks = host->pdata->quirks;
>>
>> spin_lock_init(&host->lock);
>> INIT_LIST_HEAD(&host->queue);
>>
>> -
>> host->dma_ops = host->pdata->dma_ops;
>> dw_mci_init_dma(host);
>>
>> @@ -2095,6 +2111,13 @@ err_dmaunmap:
>> regulator_disable(host->vmmc);
>> regulator_put(host->vmmc);
>> }
>> + kfree(host);
>
> what's this about?
This is wrong. It should not have been here. I will fix this. Thanks
for pointing this out.
Thanks,
Thomas.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] ARM: Exynos4: add support for GATE BLOCK clocks
Marek Szyprowski wrote:
>
> EXYNOS4_CLKGATE_BLOCK register can be used to disable the respective
> multimedia block hardware planes. It acts similar to the power domains.
> This patch adds transparent support for this method of gating of
> multimedia blocks. New clocks are added as parents to the respective
> block bus clocks. This patch has been tested on NURI and UniversalC210
> boards, which have bootloader which disable all multimedia blocks with
> both GATE BLOCK method and power domains.
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Kyungmin Park
> ---
> arch/arm/mach-exynos/clock-exynos4.c | 54
> ++
> 1 files changed, 54 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/clock-exynos4.c b/arch/arm/mach-
> exynos/clock-exynos4.c
> index 4287311..a2f8b2e 100644
> --- a/arch/arm/mach-exynos/clock-exynos4.c
> +++ b/arch/arm/mach-exynos/clock-exynos4.c
> @@ -213,6 +213,11 @@ static int exynos4_clk_dac_ctrl(struct clk *clk, int
> enable)
> return s5p_gatectrl(S5P_DAC_PHY_CONTROL, clk, enable);
> }
>
> +static int exynos4_clk_gate_block_ctrl(struct clk *clk, int enable)
> +{
> + return s5p_gatectrl(EXYNOS4_CLKGATE_BLOCK, clk, enable);
> +}
> +
> /* Core list of CMU_CPU side */
>
> static struct clksrc_clk exynos4_clk_mout_apll = {
> @@ -459,6 +464,37 @@ static struct clksrc_clk exynos4_clk_sclk_vpll = {
> .reg_src = { .reg = EXYNOS4_CLKSRC_TOP0, .shift = 8, .size = 1 },
> };
>
> +static struct clk exynos4_clk_gate_cam = {
> + .name = "cam",
> + .enable = exynos4_clk_gate_block_ctrl,
> + .ctrlbit= (1 << 0),
> +};
> +
> +static struct clk exynos4_clk_gate_tv = {
> + .name = "tv",
> + .enable = exynos4_clk_gate_block_ctrl,
> + .ctrlbit= (1 << 1),
> +};
> +
> +static struct clk exynos4_clk_gate_mfc = {
> + .name = "mfc",
> + .enable = exynos4_clk_gate_block_ctrl,
> + .ctrlbit= (1 << 2),
> +};
> +
> +static struct clk exynos4_clk_gate_lcd0 = {
> + .name = "lcd0",
> + .enable = exynos4_clk_gate_block_ctrl,
> + .ctrlbit= (1 << 4),
> +};
> +
> +static struct clk *exynos4_gate_clocks[] = {
> + &exynos4_clk_gate_cam,
> + &exynos4_clk_gate_tv,
> + &exynos4_clk_gate_mfc,
> + &exynos4_clk_gate_lcd0,
> +};
> +
> static struct clk exynos4_init_clocks_off[] = {
> {
> .name = "timers",
> @@ -470,36 +506,43 @@ static struct clk exynos4_init_clocks_off[] = {
> .devname= "s5p-mipi-csis.0",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 4),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "csis",
> .devname= "s5p-mipi-csis.1",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 5),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "jpeg",
> .id = 0,
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 6),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "fimc",
> .devname= "exynos4-fimc.0",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 0),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "fimc",
> .devname= "exynos4-fimc.1",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 1),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "fimc",
> .devname= "exynos4-fimc.2",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 2),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "fimc",
> .devname= "exynos4-fimc.3",
> .enable = exynos4_clk_ip_cam_ctrl,
> .ctrlbit= (1 << 3),
> + .parent = &exynos4_clk_gate_cam,
> }, {
> .name = "hsmmc",
> .devname= "s3c-sdhci.0",
> @@ -534,31 +577,37 @@ static struct clk exynos4_init_clocks_off[] = {
> .devname= "s5p-sdo",
> .enable = exynos4_clk_ip_tv_ctrl,
> .ctrlbit= (1 << 2),
> + .parent = &exynos4_clk_gate_tv,
> }, {
> .name = "mixer",
> .devname= "s5p-mixer",
> .enable = exynos4_clk_ip_tv_ctrl,
> .ctrlbit= (1 << 1),
> + .parent = &exynos4_clk
Re: [PATCH 1/7] mmc: dw_mmc: lookup for optional biu and ciu clocks
On 2 May 2012 15:23, Russell King - ARM Linux wrote: > On Tue, May 01, 2012 at 10:07:40PM -0700, Thomas Abraham wrote: >> Some platforms allow for clock gating and control of bus interface unit clock >> and card interface unit clock. Add support for clock lookup of optional biu >> and ciu clocks for clock gating and clock speed determination. > > As we're moving progressively towards the common clk API, which requires > the new clk_prepare/clk_unprepare calls, new code should be using the > dated API. Please update this to do so (eg, by using clk_prepare_enable() > instead of clk_enable() for the simple solution.) Better solutions > involve decoupling the two and only using clk_enable()/clk_disable() where > you really need the clock to be on. Thanks for your suggestion. I will modify this to use the clk_prepare_enable and clk_disable_unprepare calls for now. Thanks, Thomas. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] mmc: dw_mmc: lookup for optional biu and ciu clocks
On 2 May 2012 14:47, Will Newton wrote:
> On Wed, May 2, 2012 at 6:07 AM, Thomas Abraham
> wrote:
>> Some platforms allow for clock gating and control of bus interface unit clock
>> and card interface unit clock. Add support for clock lookup of optional biu
>> and ciu clocks for clock gating and clock speed determination.
>>
>> Signed-off-by: Abhilash Kesavan
>> Signed-off-by: Thomas Abraham
>> ---
>> drivers/mmc/host/dw_mmc.c | 35 +++
>> include/linux/mmc/dw_mmc.h | 4
>> 2 files changed, 35 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 1532357..036846f 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1938,19 +1938,35 @@ int dw_mci_probe(struct dw_mci *host)
>> return -ENODEV;
>> }
>>
>> - if (!host->pdata->bus_hz) {
>> + host->biu_clk = clk_get(&host->dev, "biu");
>> + if (IS_ERR(host->biu_clk))
>> + dev_info(&host->dev, "biu clock not available\n");
>> + else
>> + clk_enable(host->biu_clk);
>> +
>> + host->ciu_clk = clk_get(&host->dev, "ciu");
>> + if (IS_ERR(host->ciu_clk))
>> + dev_info(&host->dev, "ciu clock not available\n");
>
> Should these printks be at debug level? I'm not 100% sure either way
> but it could be a little noisy on systems that do not use these
> clocks.
Right. I will change dev_info to dev_dbg.
>
>> + else
>> + clk_enable(host->ciu_clk);
>> +
>> + if (IS_ERR(host->ciu_clk))
>> + host->bus_hz = host->pdata->bus_hz;
>> + else
>> + host->bus_hz = clk_get_rate(host->ciu_clk);
>> +
>> + if (!host->bus_hz) {
>> dev_err(&host->dev,
>> "Platform data must supply bus speed\n");
>> - return -ENODEV;
>> + ret = -ENODEV;
>> + goto err_clk;
>> }
>>
>> - host->bus_hz = host->pdata->bus_hz;
>> host->quirks = host->pdata->quirks;
>>
>> spin_lock_init(&host->lock);
>> INIT_LIST_HEAD(&host->queue);
>>
>> -
>> host->dma_ops = host->pdata->dma_ops;
>> dw_mci_init_dma(host);
>>
>> @@ -2095,6 +2111,13 @@ err_dmaunmap:
>> regulator_disable(host->vmmc);
>> regulator_put(host->vmmc);
>> }
>> + kfree(host);
>> +
>> +err_clk:
>> + clk_disable(host->ciu_clk);
>> + clk_disable(host->biu_clk);
>> + clk_put(host->ciu_clk);
>> + clk_put(host->biu_clk);
>
> Are these calls safe, or do we need to check IS_ERR as above?
The clk_disable is safe on Samsung platforms but it cannot be assumed
for other platforms. So, the IS_ERR check will be added before
clk_disable. clk_put will not need the IS_ERR check. Thanks for
pointing this out.
>
>> return ret;
>> }
>> EXPORT_SYMBOL(dw_mci_probe);
>> @@ -2128,6 +2151,10 @@ void dw_mci_remove(struct dw_mci *host)
>> regulator_put(host->vmmc);
>> }
>>
>> + clk_disable(host->ciu_clk);
>> + clk_disable(host->biu_clk);
>> + clk_put(host->ciu_clk);
>> + clk_put(host->biu_clk);
>
> Likewise.
Yes, will be fixed.
Thanks,
Thomas.
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] ARM: Exynos4: read initial state of power domain from hw registers
Marek Szyprowski wrote:
>
> Some bootloaders disable unused power domains, so kernel code should
> read the actual state from the hardware registers instead of assuming
> that their initial state is 'on'.
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Kyungmin Park
> ---
> arch/arm/mach-exynos/pm_domains.c |9 ++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pm_domains.c b/arch/arm/mach-
> exynos/pm_domains.c
> index 13b3068..26fc47d 100644
> --- a/arch/arm/mach-exynos/pm_domains.c
> +++ b/arch/arm/mach-exynos/pm_domains.c
> @@ -151,9 +151,12 @@ static __init int exynos4_pm_init_power_domain(void)
> if (of_have_populated_dt())
> return exynos_pm_dt_parse_domains();
>
> - for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++)
> - pm_genpd_init(&exynos4_pm_domains[idx]->pd, NULL,
> - exynos4_pm_domains[idx]->is_off);
> + for (idx = 0; idx < ARRAY_SIZE(exynos4_pm_domains); idx++) {
> + struct exynos_pm_domain *pd = exynos4_pm_domains[idx];
> + int on = __raw_readl(pd->base + 0x4) & S5P_INT_LOCAL_PWR_EN;
> +
> + pm_genpd_init(&pd->pd, NULL, !on);
> + }
>
> #ifdef CONFIG_S5P_DEV_FIMD0
> exynos_pm_add_dev_to_genpd(&s5p_device_fimd0, &exynos4_pd_lcd0);
> --
> 1.7.1.569.g6f426
As you said, if bootloader changed some hardware setting which is not
expected value in kernel, it should be notified to kernel. But for
pm_domaine->is_off, it should be via DT on EXYNOS now. As you know, the
functionality has been already implemented on dt.
Thanks.
Best regards,
Kgene.
--
Kukjin Kim , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] regulator: Add support for MAX77686.
Hi Sylwester,
On Thu, May 10, 2012 at 1:24 AM, Sylwester Nawrocki wrote:
> Hi,
>
> just a few nitpicks...
>
>
> On 05/09/2012 06:24 PM, Yadwinder Singh wrote:
>>
>> From: Yadwinder Singh Brar
>>
>> Add support for PMIC/regulator portion of MAX77686 multifunction device.
>> MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of
>> driver
>> which supports setting and getting the voltage of a regulator with I2C
>> interface.
>>
>> Signed-off-by: Yadwinder Singh Brar
>
> ...
>
>> diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
>> new file mode 100644
>> index 000..4aa9722
>> --- /dev/null
>> +++ b/drivers/regulator/max77686.c
>> @@ -0,0 +1,660 @@
>> +/*
>> + * max77686.c - Regulator driver for the Maxim 77686
>> + *
>> + * Copyright (C) 2012 Samsung Electronics
>
>
> I believe this should read:
>
> + * Copyright (C) 2012 Samsung Electronics Co., Ltd.
>
> In patch 1/2 the copyright notice is also not exactly correct.
>
>
>> + * Chiwoong Byun
>
>
> s/smasung/samsung
>
> ...
>
Thanks for pointing. I will rectify above mistakes.
>> +static int max77686_get_enable_register(struct regulator_dev *rdev,
>> + int *reg, int *mask, int *pattern)
>> +{
>> + int rid = rdev_get_id(rdev);
>> +
>> + switch (rid) {
>> + case MAX77686_LDO1...MAX77686_LDO26:
>> + *reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
>> + *mask = 0xC0;
>> + *pattern = 0xC0;
>
>
> What about using lower case for all hex numbers ?
>
> ...
>
Yes, i will take care of this.
>> +static int max77686_get_voltage_register(struct regulator_dev *rdev,
>> + int *_reg, int *_shift, int
>> *_mask)
>> +{
>
> ...
>
>> + case MAX77686_BUCK2:
>> + reg = MAX77686_REG_BUCK2DVS1;
>> + mask = 0xff;
>> + break;
>> + case MAX77686_BUCK3:
>> + reg = MAX77686_REG_BUCK3DVS1;
>> + mask = 0xff;
>> + break;
>> + case MAX77686_BUCK4:
>> + reg = MAX77686_REG_BUCK4DVS1;
>> + mask = 0xff;
>> + break;
>> + case MAX77686_BUCK5...MAX77686_BUCK9:
>> + reg = MAX77686_REG_BUCK5OUT + (rid - MAX77686_BUCK5) * 2;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + *_reg = reg;
>> + *_shift = shift;
>> + *_mask = mask;
>> +
>> + return 0;
>> +}
>
>
> Thanks,
> Sylwester
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Thanks,
Yadwinder.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] mfd: Add support for MAX77686.
Hi Mark,
On Wed, May 9, 2012 at 11:57 PM, Mark Brown
wrote:
> On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:
>
>> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
>> +{
>> + int ret;
>> +
>> + ret = i2c_smbus_read_byte_data(i2c, reg);
>> + if (ret < 0)
>
> It would really be better if this used the regmap API - the regulator
> API is starting to build out functionality on top of regmap which this
> won't be able to take advantage of if it doesn't use regmap.
>
I agree, I will implement it.
>> + if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
>> + pd->wakeup = true;
>
> You haven't included any binding documentatiojn for the device tree
> bindings - you should do this.
>
Ok. I will add documentation in next version of patch.
>> + max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
>> + if (max77686 == NULL) {
>> + dev_err(max77686->dev, "could not allocate memory\n");
>> + return -ENOMEM;
>> + }
>
> devm_kzalloc().
>
yes, its better option.
>> + device_init_wakeup(max77686->dev, max77686->wakeup);
>
> Why is this not just done unconditionally? There's sysfs files to allow
> userspace control.
>
yes, i agree with you. i will do it.
>> + if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
>> + ret = -EIO;
>> + dbg_info("%s : device not found on this channel\n", __func__);
>> + goto err_mfd;
>> + } else
>> + dev_info(max77686->dev, "device found\n");
>
> You should verify that the device ID you read back is the expected one.
>
Ok, I will do that also.
>> + dev_err(max77686->dev, "device probe failed : %d\n", ret);
>
> Driver core should do this for you.
>
Yes, I will remove this message.
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Thanks,
Yadwinder.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] regulator: Add support for MAX77686.
Hi Mark,
On Thu, May 10, 2012 at 12:17 AM, Mark Brown
wrote:
> On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
>
>> +/* Voltage maps in mV */
>> +static const struct voltage_map_desc ldo_voltage_map_desc = {
>> + .min = 800, .max = 3950, .step = 50, .n_bits = 6,
>> +}; /* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */
>
> Hrm, funnily enough I was just thinking about factoring this stuff out
> into the core after a conversation with Graeme Gregory the other week.
> Let's do that...
>
>> + [MAX77686_EN32KHZ_AP] = NULL,
>> + [MAX77686_EN32KHZ_CP] = NULL,
>
> Now that the generic clock API is in mainline these should be moved over
> to use it.
>
Sorry, I cann't get your point here. Please explain it little bit more.
>> +static int max77686_get_voltage_unit(int rid)
>> +{
>> + int unit = 0;
>> +
>> + switch (rid) {
>> + case MAX77686_BUCK2...MAX77686_BUCK4:
>> + unit = 1; /* BUCK2,3,4 is uV */
>> + break;
>> + default:
>> + unit = 1000;
>
> Why not just list everything in uV?
>
Yes, everything should be in uV, I will correct it.
>> +static int max77686_get_voltage(struct regulator_dev *rdev)
>> +{
>
> Implement get_voltage_sel().
>
>> +static inline int max77686_get_voltage_proper_val(const struct
>> voltage_map_desc
>> + *desc, int min_vol,
>> + int max_vol)
>> +{
>> + int i = 0;
>> +
>> + if (desc == NULL)
>> + return -EINVAL;
>> +
>> + if (max_vol < desc->min || min_vol > desc->max)
>> + return -EINVAL;
>> +
>> + while (desc->min + desc->step * i < min_vol &&
>> + desc->min + desc->step * i < desc->max)
>> + i++;
>
> Why are you iterating here? Calculate! Though like I say let's factor
> this out anyway.
>
Yes, I will do it.
>> + if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
>> + rid == MAX77686_BUCK4) {
>> + /* If the voltage is increasing */
>> + if (org < i)
>> + udelay(DIV_ROUND_UP(desc->step * (i - org),
>> + ramp[max77686->ramp_delay]));
>> + }
>
> Don't do this, implement set_voltage_time_sel().
>
Ok, I will implement it.
>> + .enable = max77686_reg_enable,
>> + .disable = max77686_reg_disable,
>> + .set_suspend_enable = max77686_reg_enable,
>> + .set_suspend_disable = max77686_reg_disable,
>
> You've got the same ops for suspend and non-suspend cases here, this is
> clearly buggy.
>
>> +/* count the number of regulators to be supported in pmic */
>> + pdata->num_regulators = 0;
>
> Coding style.
>
>> + if (iodev->dev->of_node) {
>> + ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
>> + if (ret)
>> + return ret;
>
> This ought to use of_regulator_match().
>
Ok, I will look into it.
>> + }
>> +
>> + if (!pdata) {
>> + dev_err(&pdev->dev, "platform data not found\n");
>> + return -ENODEV;
>> + }
>
> This should be totally fine.
>
I will look into it.
>> + max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
>> + if (!max77686)
>> + return -ENOMEM;
>
> devm_kzalloc().
>
Yes, its better option.
>> + if (pdata->ramp_delay) {
>> + max77686->ramp_delay = pdata->ramp_delay;
>> + max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> + RAMP_VALUE, RAMP_MASK);
>
> This appears not to actually use the value passed in as platform_data.
>
It gets corresponding index of ramp_rate value in ramp_rate_value
table supported by hardware, from platform_data which we write to
ramp_rate control bits of control registers.
>> +
>> + for (i = 0; i < pdata->num_regulators; i++) {
>> + const struct voltage_map_desc *desc;
>> + int id = pdata->regulators[i].id;
>> +
>> + desc = reg_voltage_map[id];
>> + if (desc)
>> + regulators[id].n_voltages =
>> + (desc->max - desc->min) / desc->step + 1;
>> +
>> + rdev[i] = regulator_register(®ulators[id], max77686->dev,
>> + pdata->regulators[i].initdata,
>> + max77686, NULL);
>
> No, you should unconditionally register all regulators the device
> physically has. This is useful for debug and simplifies the code.
>
Ok. I will do it.
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Thanks,
Yadwinder.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
