[patch] drm/exynos: change zero to NULL for sparse

[Dan Carpenter]

We recently changed this function to return a pointer instead of an int
so we need to change this zero to a NULL or Sparse complains:

drivers/gpu/drm/exynos/exynos_drm_drv.h:346:47:
warning: Using plain integer as NULL pointer

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h 
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 36535f3..06cde45 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -343,7 +343,7 @@ struct exynos_drm_display * exynos_dpi_probe(struct device 
*dev);
 int exynos_dpi_remove(struct device *dev);
 #else
 static inline struct exynos_drm_display *
-exynos_dpi_probe(struct device *dev) { return 0; }
+exynos_dpi_probe(struct device *dev) { return NULL; }
 static inline int exynos_dpi_remove(struct device *dev) { return 0; }
 #endif
 
--
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 1/2 v3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Chatradhi]

Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
defined under "controller-data" node under each slave node.

&spi_x {
cs-gpios <>;
...
slave_node {

controller-data {
cs-gpio = <>;
...
};
...
};
...
};

Where as, SPI core and many other drivers uses "cs-gpios" for
from device tree node.

Hence, make changes in spi-s3c64xx.c driver to make use of
"cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
slaves "controller-data"(child) node.

Also updates the Device tree Documentation.

Signed-off-by: Naveen Krishna Chatradhi 
Acked-by: Rob Herring 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
Cc: Tomasz Figa 
---
Changes since v2:
1. updated the gpios usage in Documentation
2. use the spi->cs_gpio in the driver, instead of parsing the node again.
3. Corrected error check of the of.node and during gpio_free

 .../devicetree/bindings/spi/spi-samsung.txt|8 +++-
 drivers/spi/spi-s3c64xx.c  |   18 ++
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt 
b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 86aa061..2d29dac 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -42,15 +42,13 @@ Optional Board Specific Properties:
 - num-cs: Specifies the number of chip select lines supported. If
   not specified, the default number of chip select lines is set to 1.
 
+- cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is 
required
   by the spi controller.
 
-  - cs-gpio: A gpio specifier that specifies the gpio line used as
-the slave select line by the spi controller. The format of the gpio
-specifier depends on the gpio controller.
-
   - samsung,spi-feedback-delay: The sampling phase shift to be applied on the
 miso line (to account for any lag in the miso line). The following are the
 valid values.
@@ -85,6 +83,7 @@ Example:
#size-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&spi0_bus>;
+   cs-gpios = <&gpa2 5 0>;
 
w25q80bw@0 {
#address-cells = <1>;
@@ -94,7 +93,6 @@ Example:
spi-max-frequency = <1>;
 
controller-data {
-   cs-gpio = <&gpa2 5 1 0 3>;
samsung,spi-feedback-delay = <0>;
};
 
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 75a5696..842b148 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -772,24 +772,19 @@ static struct s3c64xx_spi_csinfo 
*s3c64xx_get_slave_ctrldata(
 
cs = kzalloc(sizeof(*cs), GFP_KERNEL);
if (!cs) {
-   of_node_put(data_np);
return ERR_PTR(-ENOMEM);
}
 
-   /* The CS line is asserted/deasserted by the gpio pin */
-   if (sdd->cs_gpio)
-   cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
-
-   if (!gpio_is_valid(cs->line)) {
+   if (!gpio_is_valid(spi->cs_gpio)) {
dev_err(&spi->dev, "chip select gpio is not specified or 
invalid\n");
-   kfree(cs);
-   of_node_put(data_np);
return ERR_PTR(-EINVAL);
}
+   cs->line = spi->cs_gpio;
 
of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
cs->fb_delay = fb_delay;
of_node_put(data_np);
+
return cs;
 }
 
@@ -828,8 +823,6 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
cs->line, err);
goto err_gpio_req;
}
-
-   spi->cs_gpio = cs->line;
}
 
spi_set_ctldata(spi, cs);
@@ -884,7 +877,8 @@ setup_exit:
/* setup() returns with device de-selected */
writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
-   gpio_free(cs->line);
+   if (cs->line)
+   gpio_free(cs->line);
spi_set_ctldata(spi, NULL);
 
 err_gpio_req:
@@ -1077,7 +1071,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
sdd->sfr_start = mem_res->start;
sdd->cs_gpio = true;
if (pdev->dev.of_node) {
-   if (!of_find_property(pdev->dev.of_node, "cs-gpio", NULL))
+   if (!of_find_property(pdev->dev.of_node, "cs-gpios", NULL))
sdd->cs_gpio = false;
 
ret = of_alias_get_id(pdev->dev.of_node, "spi");
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubsc

[PATCH 2/2 v3] ARM: DTS: move "cs-gpio" from "controller-data" to under spi node

[Naveen Krishna Chatradhi]

This patch moves the "cs-gpio" field from "controller-data" child
node to under the spi device node.

Respective changes are preposed to spi-s3c64xx.c driver.

Signed-off-by: Naveen Krishna Chatradhi 
Acked-by: Rob Herring 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
Cc: Tomasz Figa 
---
Changes since v2:
None

 arch/arm/boot/dts/exynos4210-smdkv310.dts |2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts   |2 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts 
b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 636d166..9191491 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -169,6 +169,7 @@
 
spi_2: spi@1394 {
status = "okay";
+   cs-gpios = <&gpc1 2 0>;
 
w25x80@0 {
#address-cells = <1>;
@@ -178,7 +179,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpc1 2 0>;
samsung,spi-feedback-delay = <0>;
};
 
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts 
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 8a558b7..204b0de 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -512,6 +512,7 @@
spi_1: spi@1393 {
pinctrl-names = "default";
pinctrl-0 = <&spi1_bus>;
+   cs-gpios = <&gpb 5 0>;
status = "okay";
 
s5c73m3_spi: s5c73m3 {
@@ -519,7 +520,6 @@
spi-max-frequency = <5000>;
reg = <0>;
controller-data {
-   cs-gpio = <&gpb 5 0>;
samsung,spi-feedback-delay = <2>;
};
};
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts 
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a794a70..0c6433a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -316,6 +316,7 @@
};
 
spi_1: spi@12d3 {
+   cs-gpios = <&gpa2 5 0>;
status = "okay";
 
w25q80bw@0 {
@@ -326,7 +327,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpa2 5 0>;
samsung,spi-feedback-delay = <0>;
};
 
-- 
1.7.9.5

--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Ch]

Hello Tomasz,

On 11 June 2014 01:19, Tomasz Figa  wrote:
> Hi Naveen,
>
> On 10.06.2014 12:08, Naveen Krishna Chatradhi wrote:
>> Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
>> defined under "controller-data" node under each slave node.
>
> [snip]
>
>> @@ -85,6 +83,7 @@ Example:
>>   #size-cells = <0>;
>>   pinctrl-names = "default";
>>   pinctrl-0 = <&spi0_bus>;
>> + cs-gpios = <&gpa2 5 1 0 3>;
>
> While at it, you might also update the example to something more
> appropriate on Samsung platforms, e.g. <&gpa2 5 0>;
Sylwester gave this comment, will club with the other changes.
>
>>
>>   w25q80bw@0 {
>>   #address-cells = <1>;
>
> [snip]
>
>> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
>> *spi)
>> +{
>> + struct device_node *parent_np = NULL;
>> + struct s3c64xx_spi_driver_data *sdd;
>> + struct s3c64xx_spi_csinfo *cs;
>> +
>> + parent_np = of_get_parent(spi->dev.of_node);
>> + if (!parent_np) {
>> + dev_err(&spi->dev, "Parent node not found\n");
>>   return ERR_PTR(-EINVAL);
>>   }
>>
>> + sdd = spi_master_get_devdata(spi->master);
>> +
>>   cs = kzalloc(sizeof(*cs), GFP_KERNEL);
>>   if (!cs) {
>> - of_node_put(data_np);
>> + of_node_put(parent_np);
>>   return ERR_PTR(-ENOMEM);
>>   }
>>
>>   /* The CS line is asserted/deasserted by the gpio pin */
>>   if (sdd->cs_gpio)
>> - cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
>> + cs->line = of_get_named_gpio(parent_np, "cs-gpios", 0);
>
> This is wrong. The "cs-gpios" property is supposed to be an array,
> indexed by chip select number of SPI devices (indicated by their "reg"
> properties).
>
> Moreover, is there a need to parse this manually in this driver? I can
> see respective parsing code in of_spi_register_master().

Right, spi_add_device() parses the "cs-gpios" and gives the "spi->cs_gpio"
before calling the driver setup() function.

Will respin with appropriate changes.
>
> Best regards,
> Tomasz



-- 
Shine bright,
(: Nav :)
--
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: drm/exynos: consider deferred probe case

[Inki Dae]

Hello Dan,

On 2014년 06월 10일 21:38, Dan Carpenter wrote:
> Hello Inki Dae,
> 
> The patch df5225bc9a87: "drm/exynos: consider deferred probe case"
> from May 29, 2014, leads to the following static checker warning:
> 
>   drivers/gpu/drm/exynos/exynos_drm_fimd.c:996 fimd_probe()
>   warn: 'ctx->display' isn't an ERR_PTR
> 
> drivers/gpu/drm/exynos/exynos_drm_fimd.c
>994  
>995  ctx->display = exynos_dpi_probe(dev);
>996  if (IS_ERR(ctx->display))
>997  return PTR_ERR(ctx->display);
>998  
> 
> Smatch is complaining because my config has CONFIG_DRM_EXYNOS_DPI
> disabled.
> 
> 1) If CONFIG_DRM_EXYNOS_DPI isn't enabled, we still return "0".  That
> will cause a Sparse warning.
>

It would be no problem. This code will return PTR_ERR(ctx->dislay) only
in case that dpi module is enabled.

> 2) Also there are still a number of checks for "if (ctx->display)".
> Those things are weird to me, are those checks to see
> CONFIG_DRM_EXYNOS_DPI is enabled or are they checking that
> exynos_dpi_probe() succeeded?
> 

Right. crtc driver, fimd, needs encoder/connector objects. In Exynos drm
case, encoder/connector can be created by dpi module for parallel panel,
by dsi module for mipi dsi based panel, or by dp module for eDP based panel.

So if some board has a parallel panel, encoder/connector objects will be
created by dpi module, and then fimd module will refer to ctx->display
object to control the parallel panel device. Otherwise, dsi or dp
modules will create encoder/connector objects in its driver, and one of
them will use its own display object to control corresponding panel device.

Thanks,
Inki Dae

> regards,
> dan carpenter
> --
> 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 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Ch]

Hello Doug,

On 10 June 2014 23:56, Doug Anderson  wrote:
> Naveen,
>
> Not a full review, but a few quick things I happened to notice:
>
> On Tue, Jun 10, 2014 at 3:08 AM, Naveen Krishna Chatradhi
>  wrote:
>> @@ -94,7 +93,6 @@ Example:
>> spi-max-frequency = <1>;
>>
>> controller-data {
>> -   cs-gpio = <&gpa2 5 1 0 3>;
>> samsung,spi-feedback-delay = <0>;
>
> In a future patch (not this one!) it might make sense to also move
> spi-feedback-delay out.
After this change, "cs->line" in the struct s3c64xx_spi_csinfo {} is useless.
Will take others opinion and remove "fb_delay" too.
>
>
>> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
>> *spi)
>
> I believe that you can make use of the fact that the SPI core already
> got these GPIOs for you.  See of_spi_register_master().  Everything
> you need ought to be in master->num_chipselect and master->cs_gpios.
cs->line is a duplicate and can be replaced with master->cs_gpios from SPI core.
>
> Strangely, it doesn't look like any other drivers use this, so
> possibly I'm confused...
>
>
>> @@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>> struct s3c64xx_spi_info *sci;
>> int err;
>>
>> +   if (!spi->dev.of_node) {
>> +   dev_err(&spi->dev, "device node not found\n");
>> +   return -EINVAL;
>> +   }
>
> This seems incredibly broken.  You're saying that the SPI driver no
> longer works without the device tree?  I don't think that's desirable,
> but I suppose I could be wrong.
I will move this check to the s3c64xx_get_slave_ctrldata() function.


I guess the non-DT version of the driver is broken too.
For non-DT driver sdd->cs_gpio is true, but the cs->line is never initialized.
>
> Also note that you still left an "if" test below for trying to deal
> with the fact that there is no "of_node".

>
>
> -Doug



-- 
Shine bright,
(: Nav :)
--
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: exynos5420-peach-pi: linux-next boot fails unless mau_epll left enabled?

[Tushar Behera]

On 06/11/2014 01:56 AM, Doug Anderson wrote:
> Javier,
> 
> On Tue, Jun 10, 2014 at 1:03 PM, Javier Martinez Canillas
>  wrote:
>> Yes, I did not have this issue before. However... I installed the latest 
>> Peach
>> pit recovery image you provided me and mainline kernel started to hang on 
>> boot.
>> I remembered this thread so tested Kevin's patch which make it boot again.
>>
>> You said that chain-booting u-boot runs the sound init code but did you try
>> booting a signed FIT image using the verified u-boot?
> 
> Right, I'd bet that it's actually verified boot that's calling the
> sound init code (probably just in case it needs to beep at the user).
> ...so this would happen even without chain booting.  I haven't tried
> signing / booting without chain booting, but I can totally believe it
> would hang just as easily.
> 
> 
>> I'm asking because I'm see the hang both when chain-booting nv-u-boot from a
>> kernel partition and booting a signed FIT image with the verified u-boot.
>>
>> So it seems this is a regression in latest U-boot and I didn't run into this
>> before due having an older RW U-Boot on my Peach pit?
> 
> You must have had a very old RW U-Boot, possibly one before audio
> support was added.  I think we just need to fix this in the kernel so
> it doesn't hang.
> 
> -Doug
> 

Hi Kevin,

I have posted the fixes[1] in the mailing list. Please test and update.

Changes:
1 patch in drivers/clk/samsung/clk-exynos-audss.c
1 patch in arch/arm/boot/dts/exynos5420.dtsi

For testing audio in Peach-pi,
1 patch in arch/arm/boot/dts/exynos5800-peach-pi.dts

[1] [PATCH 0/3] Fix boot-hang on Peach-pit and Enable audio

-- 
Tushar Behera
--
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 2/3] ARM: dts: Update the parent for Audss clocks in Exynos5420

[Tushar Behera]

Currently CLK_FOUT_EPLL was set as one of the parents of AUDSS mux.
As per the user manual, it should be CLK_MAU_EPLL.

The problem surfaced when the bootloader in Peach-pit board set
the EPLL clock as the parent of AUDSS mux. While booting the kernel,
we used to get a system hang during late boot if CLK_MAU_EPLL was
disabled.

Signed-off-by: Tushar Behera 
Signed-off-by: Shaik Ameer Basha 
Reported-by: Kevin Hilman 
---
 arch/arm/boot/dts/exynos5420.dtsi |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
b/arch/arm/boot/dts/exynos5420.dtsi
index e385322..79e9119 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -167,7 +167,7 @@
compatible = "samsung,exynos5420-audss-clock";
reg = <0x0381 0x0C>;
#clock-cells = <1>;
-   clocks = <&clock CLK_FIN_PLL>, <&clock CLK_FOUT_EPLL>,
+   clocks = <&clock CLK_FIN_PLL>, <&clock CLK_MAU_EPLL>,
 <&clock CLK_SCLK_MAUDIO0>, <&clock CLK_SCLK_MAUPCM0>;
clock-names = "pll_ref", "pll_in", "sclk_audio", "sclk_pcm_in";
};
-- 
1.7.9.5

--
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 3/3] ARM: dts: Enable audio support for Peach-pi board

[Tushar Behera]

Peach-pi board has MAX98090 audio codec connected on HSI2C-7 bus.

Signed-off-by: Tushar Behera 
---
 arch/arm/boot/dts/exynos5800-peach-pi.dts |   31 +
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts 
b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index f3af207..76f5966 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -78,9 +78,27 @@
pinctrl-0 = <&usb301_vbus_en>;
enable-active-high;
};
+
+   sound {
+   compatible = "google,snow-audio-max98090";
+
+   samsung,i2s-controller = <&i2s0>;
+   samsung,audio-codec = <&max98090>;
+   };
+};
+
+&i2s0 {
+   status = "okay";
 };
 
 &pinctrl_0 {
+   max98090_irq: max98090-irq {
+   samsung,pins = "gpx0-2";
+   samsung,pin-function = <0>;
+   samsung,pin-pud = <0>;
+   samsung,pin-drv = <0>;
+   };
+
tpm_irq: tpm-irq {
samsung,pins = "gpx1-0";
samsung,pin-function = <0>;
@@ -207,6 +225,19 @@
samsung,invert-vclk;
 };
 
+&hsi2c_7 {
+   status = "okay";
+
+   max98090: codec@10 {
+   compatible = "maxim,max98090";
+   reg = <0x10>;
+   interrupts = <2 0>;
+   interrupt-parent = <&gpx0>;
+   pinctrl-names = "default";
+   pinctrl-0 = <&max98090_irq>;
+   };
+};
+
 &hsi2c_9 {
status = "okay";
clock-frequency = <40>;
-- 
1.7.9.5

--
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 1/3] clk: exynos-audss: Keep the parent of mout_audss always enabled

[Tushar Behera]

When the output clock of AUDSS mux is disabled, we are getting kernel
oops while doing a clk_get() on other clocks provided by AUDSS. Though
user manual doesn't specify this dependency, we came across this issue
while disabling the parent of AUDSS mux clocks.

Keeping the parents of AUDSS mux always enabled fixes this issue.

Signed-off-by: Tushar Behera 
Signed-off-by: Shaik Ameer Basha 
---
 drivers/clk/samsung/clk-exynos-audss.c |   17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos-audss.c 
b/drivers/clk/samsung/clk-exynos-audss.c
index 13eae14c..1542f30 100644
--- a/drivers/clk/samsung/clk-exynos-audss.c
+++ b/drivers/clk/samsung/clk-exynos-audss.c
@@ -30,6 +30,8 @@ static struct clk **clk_table;
 static void __iomem *reg_base;
 static struct clk_onecell_data clk_data;
 
+static struct clk *pll_ref, *pll_in;
+
 #define ASS_CLK_SRC 0x0
 #define ASS_CLK_DIV 0x4
 #define ASS_CLK_GATE 0x8
@@ -83,7 +85,7 @@ static int exynos_audss_clk_probe(struct platform_device 
*pdev)
const char *mout_audss_p[] = {"fin_pll", "fout_epll"};
const char *mout_i2s_p[] = {"mout_audss", "cdclk0", "sclk_audio0"};
const char *sclk_pcm_p = "sclk_pcm0";
-   struct clk *pll_ref, *pll_in, *cdclk, *sclk_audio, *sclk_pcm_in;
+   struct clk *cdclk, *sclk_audio, *sclk_pcm_in;
const struct of_device_id *match;
enum exynos_audss_clk_type variant;
 
@@ -113,10 +115,14 @@ static int exynos_audss_clk_probe(struct platform_device 
*pdev)
 
pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
pll_in = devm_clk_get(&pdev->dev, "pll_in");
-   if (!IS_ERR(pll_ref))
+   if (!IS_ERR(pll_ref)) {
mout_audss_p[0] = __clk_get_name(pll_ref);
-   if (!IS_ERR(pll_in))
+   clk_prepare_enable(pll_ref);
+   }
+   if (!IS_ERR(pll_in)) {
mout_audss_p[1] = __clk_get_name(pll_in);
+   clk_prepare_enable(pll_in);
+   }
clk_table[EXYNOS_MOUT_AUDSS] = clk_register_mux(NULL, "mout_audss",
mout_audss_p, ARRAY_SIZE(mout_audss_p),
CLK_SET_RATE_NO_REPARENT,
@@ -217,6 +223,11 @@ static int exynos_audss_clk_remove(struct platform_device 
*pdev)
clk_unregister(clk_table[i]);
}
 
+   if (!IS_ERR(pll_in))
+   clk_disable_unprepare(pll_in);
+   if (!IS_ERR(pll_ref))
+   clk_disable_unprepare(pll_ref);
+
return 0;
 }
 
-- 
1.7.9.5

--
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 0/3] Fix boot-hang on Peach-pit and Enable audio

[Tushar Behera]

With next-20140610, Peach-pit/Peach-pi board hangs during boot if we
run 'sound init' during u-boot. The issue is fixed in following patches.
While at it, also enable audio support for Peach-pi board.

How to test audio on Peach-pi:
* On top of exynos_defconfig, enable SND_SOC_SNOW and PL330_DMA.
* Run 'sound init' at u-boot prompt.

Tushar Behera (3):
  clk: exynos-audss: Keep the parent of mout_audss always enabled
  ARM: dts: Update the parent for Audss clocks in Exynos5420
  ARM: dts: Enable audio support for Peach-pi board

 arch/arm/boot/dts/exynos5420.dtsi |2 +-
 arch/arm/boot/dts/exynos5800-peach-pi.dts |   31 +
 drivers/clk/samsung/clk-exynos-audss.c|   17 +---
 3 files changed, 46 insertions(+), 4 deletions(-)

-- 
1.7.9.5

--
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: mcpm: Don't rely on firmware's secondary_cpu_start

[Chander Kashyap]

Hi Doug,

On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre  wrote:
> On Tue, 10 Jun 2014, Doug Anderson wrote:
>
>> My S-state knowledge is not strong, but I believe that Lorenzo's
>> questions matter if we're using S2 for CPUidle (where we actually turn
>> off power and hot unplug CPUs) but not when we're using S1 for CPUidle
>> (where we just enter WFI/WFE).
>>

No Its not plain WFI.

All cores in Exynos5420 can be powered off independently.
This functionality has been tested.

Below is the link for the posted patches.

https://lkml.org/lkml/2014/6/10/194

And as Nicolas wrote, these patches need MCPM for that.

>> I believe that in ChromeOS we use S1 CPUidle and that it works fine.
>> We've never implemented S2 that I'm aware of.
>
> You'll have to rely on MCPM for that.  That's probably why it hasn't
> been implemented before.
>
>
> Nicolas
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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: linux-next on Chromebook2: DRM failing to allocate

[Rahul Sharma]

On 11 June 2014 03:48, Kevin Hilman  wrote:
> Hi Ajay,
>
> On Tue, Jun 10, 2014 at 1:51 PM, Ajay kumar  wrote:
>> Hi,
>>
>> On 6/11/14, Kevin Hilman  wrote:
>>> On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
>>>  wrote:
 On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman 
 wrote:
> I'm trying to get the latest linux-next working on my Chromebook2
> (it's booting to a serial console) and am now trying to get the
> display working (at least for a frambuffer console.)
>
> Since the display nodes seem to be present in the exynos5800-peach-pi
> DTS, I tried enabling DRM and it's failing to allocate memory (log
> below[1]
>
> Is there some additional memory setup/allocations I should be doing?
> maybe with CMA?

 Probably not CMA, but maybe you don't have the iommu enabled?
>>>
>>> Turns out it was missing CMA support.  Specifically:
>>>CONFIG_CMA=y
>>>CONFIG_DMA_CMA=y
>>> are needed (my full .config is here: http://hastebin.com/uqopirazir.vbs)
>>>
>>> With that, it allocates, appears to detect the panel and even claims
>>> "Console: switching to colour frame buffer device", but I don't see
>>> tux or any output on the display (DRM debug output below).
>>>
>>> Note that I'm chain-loading nv_uboot from an SD card, and u-boot is
>>> driving the display (black text on white background.)  As soon as it
>>> starts the kernel though, u-boot seems to shut down the display
>>> (though the backlight seems to still be on.)
>>>
>>> Maybe the DT for peach-pi is missing the regulator used to power the
>>> panel, or maybe a GPIO used to power up the panel?
>>>
>>> Any ideas?
>> Not only the DT patches, but few patches are missing to support the
>> panel present on peach-pi.
>> You should also take the following patches to be able to get the
>> display up on peach-pi:
>> http://www.spinics.net/lists/linux-samsung-soc/msg32122.html
>
> Excellent, thanks for the pointer to those patches.  I'll have a look.
>
> Can you confirm that this should work even when chain-loading
> nv_uboot?  It appears u-boot is powering down the panel.

If u-boot is powering down the panel, you also need EC and Tps DT
patches to get regulators up in kernel. Those are not posted yet. I will
send these patches to you.

Regards,
Rahul sharma.

>
> Kevin
> --
> 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: [RESEND PATCH v2] clk: exynos4: Add PPMU IP block source clocks.

[Tomasz Figa]

Hi Jonghwa,

On 11.06.2014 02:22, Jonghwa Lee wrote:
> Exynos4 has saveral PPMUs and each of them has operation clock which
> can be gated through CMU's SFR control.
> 
> New clocks are listed below. All clocks are added as a gate-typed clock.
> 
> CLK_PPMULEFT, CLK_PPMURIGHT, CLK_PPMUCAMIF, CLK_PPMUTV, CLK_PPMUMFC_L,
> CLK_PPMUMFC_R, CLK_G3D, CLK_PPMUIMAGE, CLK_PPMULCD0, CLK_PPMULCD1,
> CLK_PPMUFILE, CLK_PPMUGPS, CLK_PPMUDMC0, CLK_PPMUDMC1, CLK_PPMUCPU,
> CLK_PPMUACP,
> 
> Signed-off-by: Jonghwa Lee 
> Acked-by: Chanwoo Choi 
> ---
>  V2 :
> - Change clock definition order.
> 
>  drivers/clk/samsung/clk-exynos4.c   |   19 +++
>  include/dt-bindings/clock/exynos4.h |   18 ++
>  2 files changed, 37 insertions(+)

I have already queued the original v2 you sent, just waiting for
3.16-rc1 to be released to apply. :)

Best regards,
Tomasz
--
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

[RESEND PATCH v2] clk: exynos4: Add PPMU IP block source clocks.

[Jonghwa Lee]

Exynos4 has saveral PPMUs and each of them has operation clock which
can be gated through CMU's SFR control.

New clocks are listed below. All clocks are added as a gate-typed clock.

CLK_PPMULEFT, CLK_PPMURIGHT, CLK_PPMUCAMIF, CLK_PPMUTV, CLK_PPMUMFC_L,
CLK_PPMUMFC_R, CLK_G3D, CLK_PPMUIMAGE, CLK_PPMULCD0, CLK_PPMULCD1,
CLK_PPMUFILE, CLK_PPMUGPS, CLK_PPMUDMC0, CLK_PPMUDMC1, CLK_PPMUCPU,
CLK_PPMUACP,

Signed-off-by: Jonghwa Lee 
Acked-by: Chanwoo Choi 
---
 V2 :
- Change clock definition order.

 drivers/clk/samsung/clk-exynos4.c   |   19 +++
 include/dt-bindings/clock/exynos4.h |   18 ++
 2 files changed, 37 insertions(+)

diff --git a/drivers/clk/samsung/clk-exynos4.c 
b/drivers/clk/samsung/clk-exynos4.c
index b4f9672..bed19b9 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -680,6 +680,8 @@ static struct samsung_gate_clock exynos4_gate_clks[] 
__initdata = {
 * the device name and clock alias names specified below for some
 * of the clocks can be removed.
 */
+   GATE(CLK_PPMULEFT, "ppmuleft", "aclk200", GATE_IP_LEFTBUS, 1, 0, 0),
+   GATE(CLK_PPMURIGHT, "ppmuright", "aclk200", GATE_IP_RIGHTBUS, 1, 0, 0),
GATE(CLK_SCLK_HDMI, "sclk_hdmi", "mout_hdmi", SRC_MASK_TV, 0, 0, 0),
GATE(CLK_SCLK_SPDIF, "sclk_spdif", "mout_spdif", SRC_MASK_PERIL1, 8, 0,
0),
@@ -695,11 +697,13 @@ static struct samsung_gate_clock exynos4_gate_clks[] 
__initdata = {
GATE(CLK_SROMC, "sromc", "aclk133", GATE_IP_FSYS, 11, 0, 0),
GATE(CLK_SCLK_G3D, "sclk_g3d", "div_g3d", GATE_IP_G3D, 0,
CLK_SET_RATE_PARENT, 0),
+   GATE(CLK_PPMUG3D, "ppmug3d", "aclk200", GATE_IP_G3D, 1, 0, 0),
GATE(CLK_USB_DEVICE, "usb_device", "aclk133", GATE_IP_FSYS, 13, 0, 0),
GATE(CLK_ONENAND, "onenand", "aclk133", GATE_IP_FSYS, 15, 0, 0),
GATE(CLK_NFCON, "nfcon", "aclk133", GATE_IP_FSYS, 16, 0, 0),
GATE(CLK_GPS, "gps", "aclk133", GATE_IP_GPS, 0, 0, 0),
GATE(CLK_SMMU_GPS, "smmu_gps", "aclk133", GATE_IP_GPS, 1, 0, 0),
+   GATE(CLK_PPMUGPS, "ppmugps", "aclk200", GATE_IP_GPS, 2, 0, 0),
GATE(CLK_SLIMBUS, "slimbus", "aclk100", GATE_IP_PERIL, 25, 0, 0),
GATE(CLK_SCLK_CAM0, "sclk_cam0", "div_cam0", GATE_SCLK_CAM, 4,
CLK_SET_RATE_PARENT, 0),
@@ -781,19 +785,24 @@ static struct samsung_gate_clock exynos4_gate_clks[] 
__initdata = {
0, 0),
GATE(CLK_SMMU_JPEG, "smmu_jpeg", "aclk160", GATE_IP_CAM, 11,
0, 0),
+   GATE(CLK_PPMUCAMIF, "ppmucamif", "aclk160", GATE_IP_CAM, 16, 0, 0),
GATE(CLK_PIXELASYNCM0, "pxl_async0", "aclk160", GATE_IP_CAM, 17, 0, 0),
GATE(CLK_PIXELASYNCM1, "pxl_async1", "aclk160", GATE_IP_CAM, 18, 0, 0),
GATE(CLK_SMMU_TV, "smmu_tv", "aclk160", GATE_IP_TV, 4,
0, 0),
+   GATE(CLK_PPMUTV, "ppmutv", "aclk160", GATE_IP_TV, 5, 0, 0),
GATE(CLK_MFC, "mfc", "aclk100", GATE_IP_MFC, 0, 0, 0),
GATE(CLK_SMMU_MFCL, "smmu_mfcl", "aclk100", GATE_IP_MFC, 1,
0, 0),
GATE(CLK_SMMU_MFCR, "smmu_mfcr", "aclk100", GATE_IP_MFC, 2,
0, 0),
+   GATE(CLK_PPMUMFC_L, "ppmumfc_l", "aclk100", GATE_IP_MFC, 3, 0, 0),
+   GATE(CLK_PPMUMFC_R, "ppmumfc_r", "aclk100", GATE_IP_MFC, 4, 0, 0),
GATE(CLK_FIMD0, "fimd0", "aclk160", GATE_IP_LCD0, 0,
0, 0),
GATE(CLK_SMMU_FIMD0, "smmu_fimd0", "aclk160", GATE_IP_LCD0, 4,
0, 0),
+   GATE(CLK_PPMULCD0, "ppmulcd0", "aclk160", GATE_IP_LCD0, 5, 0, 0),
GATE(CLK_PDMA0, "pdma0", "aclk133", GATE_IP_FSYS, 0,
0, 0),
GATE(CLK_PDMA1, "pdma1", "aclk133", GATE_IP_FSYS, 1,
@@ -806,6 +815,7 @@ static struct samsung_gate_clock exynos4_gate_clks[] 
__initdata = {
0, 0),
GATE(CLK_SDMMC3, "sdmmc3", "aclk133", GATE_IP_FSYS, 8,
0, 0),
+   GATE(CLK_PPMUFILE, "ppmufile", "aclk133", GATE_IP_FSYS, 17, 0, 0),
GATE(CLK_UART0, "uart0", "aclk100", GATE_IP_PERIL, 0,
0, 0),
GATE(CLK_UART1, "uart1", "aclk100", GATE_IP_PERIL, 1,
@@ -852,6 +862,10 @@ static struct samsung_gate_clock exynos4_gate_clks[] 
__initdata = {
0, 0),
GATE(CLK_AC97, "ac97", "aclk100", GATE_IP_PERIL, 27,
0, 0),
+   GATE(CLK_PPMUDMC0, "ppmudmc0", "aclk133", GATE_IP_DMC, 8, 0, 0),
+   GATE(CLK_PPMUDMC1, "ppmudmc1", "aclk133", GATE_IP_DMC, 9, 0, 0),
+   GATE(CLK_PPMUCPU, "ppmucpu", "aclk133", GATE_IP_DMC, 10, 0, 0),
+   GATE(CLK_PPMUACP, "ppmuacp", "aclk133", GATE_IP_DMC, 16, 0, 0),
 };
 
 /* list of gate clocks supported in exynos4210 soc */
@@ -863,6 +877,9 @@ static struct samsung_gate_clock exynos4210_gate_clks[] 
__initdata = {
GATE(CLK_SMMU_G2D, "smmu_g2d", "aclk200", E4210_GATE_IP_IMAG

Re: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

[Tomasz Figa]

On 11.06.2014 01:44, Chanwoo Choi wrote:
> On 06/11/2014 08:35 AM, Tomasz Figa wrote:
>> Hi Chanwoo,
>>
>> On 11.06.2014 01:27, Chanwoo Choi wrote:
>>> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
>>> because Exynos3250 removes WFE in secure mode so that turn on automatically
>>> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
>>> to guarantee the data synchronization of command instead of IPI_WAKEUP
>>> because Exynos3250 don't have WFE mode in secue mode.
>>>
>>> Signed-off-by: Chanwoo Choi 
>>> Acked-by: Kyungmin Park 
>>> ---
>>>  arch/arm/mach-exynos/platsmp.c  | 9 -
>>>  arch/arm/mach-exynos/pm.c   | 8 ++--
>>>  arch/arm/mach-exynos/regs-pmu.h | 4 
>>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>
>> This patch seems to be unneeded with Krzysztof's patch send a while ago
>> [1]. As reported by Krzysztof, that patch apparently fixes SMP support
>> on Exynos3250 and is much smaller and less invasive.
>>
>> [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809
> 
> OK,
> But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
> EXYNOS_ARM_CORE_CONFIGURATION(cpu).
> and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
> dsb_sev(). Exynos3250 don't need
> send IPI message.

I don't know technical details about CPU boot-up on Exynos3250 as I
haven't worked too much with this platform. According to my conversation
with Krzysztof, he found S5P_CORE_AUTOWAKEUP_EN and dsb_sev() to be not
needed. Instead S5P_CORE_WAKEUP_FROM_LOCAL_CFG can be set in
EXYNOS_ARM_CORE1_STATUS and then normal arch_send_wakeup_ipi_mask()
used. He might be able to provide more details.

> 
> I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
> sending IPI message.

I believe Krzysztof was first with this kind of patch and I'd prefer his
approach as it introduces less changes. (First version posted on 15.05
[1], with my comments addressed in v2.)

[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/31614

> 
> Krzysztof's patch used of_machine_is_compatible("samsung,exynos3250") instead 
> of soc_is_exynos3250().
> Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
> next patch(v2).

IMHO, there is no significant difference between those two. Both are
bad, because they bind quirks to specific SoCs and it's hard to reuse
them for new ones - every time a quirk is reused by a new SoC, a new (||
soc_is_xxx() or || of_machine_is_compatible("xxx")) must be added.

Best regards,
Tomasz
--
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: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

[Chanwoo Choi]

On 06/11/2014 08:35 AM, Tomasz Figa wrote:
> Hi Chanwoo,
> 
> On 11.06.2014 01:27, Chanwoo Choi wrote:
>> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
>> because Exynos3250 removes WFE in secure mode so that turn on automatically
>> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
>> to guarantee the data synchronization of command instead of IPI_WAKEUP
>> because Exynos3250 don't have WFE mode in secue mode.
>>
>> Signed-off-by: Chanwoo Choi 
>> Acked-by: Kyungmin Park 
>> ---
>>  arch/arm/mach-exynos/platsmp.c  | 9 -
>>  arch/arm/mach-exynos/pm.c   | 8 ++--
>>  arch/arm/mach-exynos/regs-pmu.h | 4 
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
> 
> This patch seems to be unneeded with Krzysztof's patch send a while ago
> [1]. As reported by Krzysztof, that patch apparently fixes SMP support
> on Exynos3250 and is much smaller and less invasive.
> 
> [1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

OK,
But Krzysztof's patch didn't include set S5P_CORE_AUTOWAKEUP_EN in 
EXYNOS_ARM_CORE_CONFIGURATION(cpu).
and then use arch_send_wakeup_ipi_mask(cpumask_of(cpu)) command instead of 
dsb_sev(). Exynos3250 don't need
send IPI message.

I'll send next patch which include only S5P_CORE_AUTOWAKEUP_EN and without 
sending IPI message.

Krzysztof's patch used of_machine_is_compatible("samsung,exynos3250") instead 
of soc_is_exynos3250().
Did you agree? If you agree to use of_machine_is_compatible(), I'll use it on 
next patch(v2).

Best Regards,
Chanwoo Choi

--
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: [RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

[Tomasz Figa]

Hi Chanwoo,

On 11.06.2014 01:27, Chanwoo Choi wrote:
> This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
> because Exynos3250 removes WFE in secure mode so that turn on automatically
> after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
> to guarantee the data synchronization of command instead of IPI_WAKEUP
> because Exynos3250 don't have WFE mode in secue mode.
> 
> Signed-off-by: Chanwoo Choi 
> Acked-by: Kyungmin Park 
> ---
>  arch/arm/mach-exynos/platsmp.c  | 9 -
>  arch/arm/mach-exynos/pm.c   | 8 ++--
>  arch/arm/mach-exynos/regs-pmu.h | 4 
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 

This patch seems to be unneeded with Krzysztof's patch send a while ago
[1]. As reported by Krzysztof, that patch apparently fixes SMP support
on Exynos3250 and is much smaller and less invasive.

[1] - http://thread.gmane.org/gmane.linux.kernel.samsung-soc/32809

Best regards,
Tomasz
--
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

[RESEND PATCH] ARM: EXYNOS: Fix the sequence of secondary CPU boot for Exynos3250

[Chanwoo Choi]

This patch set AUTOWAKEUP_EN bit to ARM_CORE_CONFIGURATION register
because Exynos3250 removes WFE in secure mode so that turn on automatically
after setting CORE_LOCAL_PWR_EN. Also, This patch use dbs_sev() macro
to guarantee the data synchronization of command instead of IPI_WAKEUP
because Exynos3250 don't have WFE mode in secue mode.

Signed-off-by: Chanwoo Choi 
Acked-by: Kyungmin Park 
---
 arch/arm/mach-exynos/platsmp.c  | 9 -
 arch/arm/mach-exynos/pm.c   | 8 ++--
 arch/arm/mach-exynos/regs-pmu.h | 4 
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index ec02422..882fb84 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -149,6 +149,10 @@ static int exynos_boot_secondary(unsigned int cpu, struct 
task_struct *idle)
return -ETIMEDOUT;
}
}
+
+   if (soc_is_exynos3250())
+   __raw_writel(EXYNOS3_COREPORESET(phys_cpu), EXYNOS_SWRESET);
+
/*
 * Send the secondary CPU a soft interrupt, thereby causing
 * the boot monitor to read the system wide flags register,
@@ -182,7 +186,10 @@ static int exynos_boot_secondary(unsigned int cpu, struct 
task_struct *idle)
 
call_firmware_op(cpu_boot, phys_cpu);
 
-   arch_send_wakeup_ipi_mask(cpumask_of(cpu));
+   if (soc_is_exynos3250())
+   dsb_sev();
+   else
+   arch_send_wakeup_ipi_mask(cpumask_of(cpu));
 
if (pen_release == -1)
break;
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 87c0d34..4681f64 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -121,8 +121,12 @@ void exynos_cpu_power_down(int cpu)
  */
 void exynos_cpu_power_up(int cpu)
 {
-   __raw_writel(S5P_CORE_LOCAL_PWR_EN,
-EXYNOS_ARM_CORE_CONFIGURATION(cpu));
+   u32 core_conf = 0;
+
+   core_conf |= S5P_CORE_LOCAL_PWR_EN;
+   if (soc_is_exynos3250())
+   core_conf |= S5P_CORE_AUTOWAKEUP_EN;
+   __raw_writel(core_conf, EXYNOS_ARM_CORE_CONFIGURATION(cpu));
 }
 
 /**
diff --git a/arch/arm/mach-exynos/regs-pmu.h b/arch/arm/mach-exynos/regs-pmu.h
index 1d13b08..674dfc2 100644
--- a/arch/arm/mach-exynos/regs-pmu.h
+++ b/arch/arm/mach-exynos/regs-pmu.h
@@ -128,6 +128,7 @@
 
 #define S5P_CORE_LOCAL_PWR_EN  0x3
 #define S5P_INT_LOCAL_PWR_EN   0x7
+#define S5P_CORE_AUTOWAKEUP_EN (1 << 31)
 
 /* Only for EXYNOS4210 */
 #define S5P_CMU_CLKSTOP_LCD1_LOWPWRS5P_PMUREG(0x1154)
@@ -186,6 +187,9 @@
 #define S5P_DIS_IRQ_CORE3  S5P_PMUREG(0x1034)
 #define S5P_DIS_IRQ_CENTRAL3   S5P_PMUREG(0x1038)
 
+/* For EXYNOS3 */
+#define EXYNOS3_COREPORESET(cpu)   ((1 << 4) << cpu)
+
 /* For EXYNOS5 */
 
 #define EXYNOS5_SYS_I2C_CFG
S5P_SYSREG(0x0234)
-- 
1.8.0

--
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: linux-next on Chromebook2: DRM failing to allocate

[Kevin Hilman]

Hi Ajay,

On Tue, Jun 10, 2014 at 1:51 PM, Ajay kumar  wrote:
> Hi,
>
> On 6/11/14, Kevin Hilman  wrote:
>> On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
>>  wrote:
>>> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman 
>>> wrote:
 I'm trying to get the latest linux-next working on my Chromebook2
 (it's booting to a serial console) and am now trying to get the
 display working (at least for a frambuffer console.)

 Since the display nodes seem to be present in the exynos5800-peach-pi
 DTS, I tried enabling DRM and it's failing to allocate memory (log
 below[1]

 Is there some additional memory setup/allocations I should be doing?
 maybe with CMA?
>>>
>>> Probably not CMA, but maybe you don't have the iommu enabled?
>>
>> Turns out it was missing CMA support.  Specifically:
>>CONFIG_CMA=y
>>CONFIG_DMA_CMA=y
>> are needed (my full .config is here: http://hastebin.com/uqopirazir.vbs)
>>
>> With that, it allocates, appears to detect the panel and even claims
>> "Console: switching to colour frame buffer device", but I don't see
>> tux or any output on the display (DRM debug output below).
>>
>> Note that I'm chain-loading nv_uboot from an SD card, and u-boot is
>> driving the display (black text on white background.)  As soon as it
>> starts the kernel though, u-boot seems to shut down the display
>> (though the backlight seems to still be on.)
>>
>> Maybe the DT for peach-pi is missing the regulator used to power the
>> panel, or maybe a GPIO used to power up the panel?
>>
>> Any ideas?
> Not only the DT patches, but few patches are missing to support the
> panel present on peach-pi.
> You should also take the following patches to be able to get the
> display up on peach-pi:
> http://www.spinics.net/lists/linux-samsung-soc/msg32122.html

Excellent, thanks for the pointer to those patches.  I'll have a look.

Can you confirm that this should work even when chain-loading
nv_uboot?  It appears u-boot is powering down the panel.

Kevin
--
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] drm/exynos: dpi: Fix NULL pointer dereference with legacy bindings

[Tomasz Figa]

If there is no panel node in DT and instead display timings are provided
directly in FIMD node, there is no panel object created and ctx->panel
becomes NULL. However during Exynos DRM initialization
drm_helper_hpd_irq_event() is called, which in turns calls
exynos_dpi_detect(), which dereferences ctx->panel without a check,
causing a NULL pointer derefrence.

This patch fixes the issue by adding necessary NULL pointer check.

Signed-off-by: Tomasz Figa 
---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index f1b8587..f44bd90 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -40,7 +40,7 @@ exynos_dpi_detect(struct drm_connector *connector, bool force)
 {
struct exynos_dpi *ctx = connector_to_dpi(connector);
 
-   if (!ctx->panel->connector)
+   if (ctx->panel && !ctx->panel->connector)
drm_panel_attach(ctx->panel, &ctx->connector);
 
return connector_status_connected;
-- 
2.0.0

--
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: linux-next on Chromebook2: DRM failing to allocate

[Ajay kumar]

Hi,

On 6/11/14, Kevin Hilman  wrote:
> On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
>  wrote:
>> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman 
>> wrote:
>>> I'm trying to get the latest linux-next working on my Chromebook2
>>> (it's booting to a serial console) and am now trying to get the
>>> display working (at least for a frambuffer console.)
>>>
>>> Since the display nodes seem to be present in the exynos5800-peach-pi
>>> DTS, I tried enabling DRM and it's failing to allocate memory (log
>>> below[1]
>>>
>>> Is there some additional memory setup/allocations I should be doing?
>>> maybe with CMA?
>>
>> Probably not CMA, but maybe you don't have the iommu enabled?
>
> Turns out it was missing CMA support.  Specifically:
>CONFIG_CMA=y
>CONFIG_DMA_CMA=y
> are needed (my full .config is here: http://hastebin.com/uqopirazir.vbs)
>
> With that, it allocates, appears to detect the panel and even claims
> "Console: switching to colour frame buffer device", but I don't see
> tux or any output on the display (DRM debug output below).
>
> Note that I'm chain-loading nv_uboot from an SD card, and u-boot is
> driving the display (black text on white background.)  As soon as it
> starts the kernel though, u-boot seems to shut down the display
> (though the backlight seems to still be on.)
>
> Maybe the DT for peach-pi is missing the regulator used to power the
> panel, or maybe a GPIO used to power up the panel?
>
> Any ideas?
Not only the DT patches, but few patches are missing to support the
panel present on peach-pi.
You should also take the following patches to be able to get the
display up on peach-pi:
http://www.spinics.net/lists/linux-samsung-soc/msg32122.html

And, Arun/me will be sending all the DT changes necessary for fimd, dp
and peach-pi panel, ASAP!

Ajay

> Kevin
>
> [1] DRM output with drm.debug=0xff  (full kernel boot log here:
> http://hastebin.com/xigofelepe.vhdl
>
> [1.192850] [drm] Initialized drm 1.1.0 20060810
> [1.197676] [drm:drm_platform_init]
> [1.200224] [drm:drm_get_platform_dev]
> [1.204092] [drm:drm_minor_register]
> [1.207851] [drm:drm_minor_register] new minor assigned 64
> [1.213154] [drm:drm_minor_register]
> [1.216791] [drm:drm_minor_register]
> [1.220608] [drm:drm_minor_register] new minor assigned 0
> [1.225869] [drm:exynos_drm_encoder_create] possible_crtcs = 0x1
> [1.231820] [drm:exynos_drm_encoder_create] encoder has been created
> [1.238295] [drm:drm_sysfs_connector_add] adding "eDP-1" to sysfs
> [1.244222] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [1.250371] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [1.256953] [drm] No driver support for vblank timestamp query.
> [1.262855] [drm:drm_helper_hpd_irq_event] [CONNECTOR:15:eDP-1]
> status updated from unknown to connected
> [1.272309] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [1.278480] [drm:exynos_drm_encoder_dpms] encoder dpms: 3
> [1.283849] [drm:exynos_drm_crtc_dpms] crtc[6] mode[3]
> [1.288964] [drm:exynos_drm_crtc_dpms] desired dpms mode is same as
> previous one.
> [1.296428] [drm:drm_helper_probe_single_connector_modes_merge_bits]
> [CONNECTOR:15:eDP-1]
> [1.304588] [drm:drm_helper_probe_single_connector_modes_merge_bits]
> [CONNECTOR:15:eDP-1] probed modes :
> [1.314043] [drm:drm_mode_debug_printmodeline] Modeline
> 16:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
> 0x0
> [1.325660] [drm:drm_setup_crtcs]
> [1.329044] [drm:drm_enable_connectors] connector 15 enabled? yes
> [1.335116] [drm:drm_target_preferred] looking for cmdline mode on
> connector 15
> [1.342402] [drm:drm_target_preferred] looking for preferred mode
> on connector 15
> [1.349862] [drm:drm_target_preferred] found mode 1920x1080
> [1.355414] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [1.361749] [drm:drm_setup_crtcs] desired mode 1920x1080 set on crtc 6
> [1.368257] [drm:exynos_drm_fbdev_create] surface width(1920),
> height(1080) and bpp(32
> [1.376150] [drm:exynos_drm_init_buf] desired size = 0x7e9000
> [1.381886] [drm:exynos_drm_gem_init] created file object = 0xec7e2000
> [1.391732] [drm:lowlevel_buffer_allocate] dma_addr(0x8e90),
> size(0x7e9000)
> [1.397581] [drm:drm_framebuffer_reference] FB ID: 18
> [1.402614] [drm:exynos_drm_fb_buffer] dma_addr = 0x8e90
> [1.408526] [drm:drm_crtc_helper_set_config]
> [1.408531] [drm:drm_crtc_helper_set_config] [CRTC:6] [FB:18]
> #connectors=1 (x y) (0 0)
> [1.408536] [drm:drm_crtc_helper_set_config] crtc has no fb, full mode
> set
> [1.408539] [drm:drm_crtc_helper_set_config] modes are different,
> full mode set
> [1.408544] [drm:drm_mode_debug_printmodeline] Modeline 0:"" 0 0 0
> 0 0 0 0 0 0 0 0x0 0x0
> [1.408549] [drm:drm_mode_debug_printmodeline] Modeline
> 17:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
> 0x0
> [   

Re: [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Rob Herring]

On Tue, Jun 10, 2014 at 1:09 PM, Doug Anderson  wrote:
> Naveen / Sylwester,
>
> On Tue, Jun 10, 2014 at 4:00 AM, Naveen Krishna Ch
>  wrote:
>>> Can we support both "cs-gpio" and "cs-gpios" for backward compatibility ?
>>> After your change all DTBs using the original pattern will not work with
>>> new kernels any more. At least I would expect such backward compatibility
>>> maintained for few kernel releases.
>>
>> The reason behind removing the "cs-gpio" or not providing backward
>> compatibility was
>>
>> 1. Since spi-core started using "cs-gpios" string from spi device node
>> several months ago.
>>   The spi-s3c64xx.c driver is partially broken for more than 6 months.
>>
>> 2. Supporting "cs-gpio" would add extra bit of code.
>>
>> I've corrected the dts files that were using "cs-gpio" under
>> "controller-data"(child node)
>> to use "cs-gpio" from spi device node (parent node).
>>
>> I will make another version if you insist.
>
> Yup, as near as I can tell this has been broken since (3146bee spi:
> s3c64xx: Added provision for dedicated cs pin).  That landed June 25th
> of 2013 into the SPI tree.
>
> ...so clearly nobody was really testing Samsung's SPI driver on ToT
> Linux.  1 year of unnoticed brokenness seems like enough time that we
> could do away with the old code.  If someone comes out of the woodwork
> and claims a dire need to support the old binding then it can be added
> then.
>
> In-tree users of this appear to be:
>
> arch/arm/boot/dts/exynos4210-smdkv310.dts:
>  cs-gpio = <&gpc1 2 0>;
> arch/arm/boot/dts/exynos4412-trats2.dts:
>  cs-gpio = <&gpb 5 0>;
> arch/arm/boot/dts/exynos5250-smdk5250.dts:
>  cs-gpio = <&gpa2 5 0>;
>
> ...and I guess nobody has bothered using the SPI flash on those boards
> for the last year?

I always try to warn people if they are breaking compatibility, but in
this case I agree it is okay. Presumably no one expects BSD or vxworks
support for this platform either?

For the binding change:

Acked-by: Rob Herring 

Rob
--
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: exynos5420-peach-pi: linux-next boot fails unless mau_epll left enabled?

[Doug Anderson]

Javier,

On Tue, Jun 10, 2014 at 1:03 PM, Javier Martinez Canillas
 wrote:
> Yes, I did not have this issue before. However... I installed the latest Peach
> pit recovery image you provided me and mainline kernel started to hang on 
> boot.
> I remembered this thread so tested Kevin's patch which make it boot again.
>
> You said that chain-booting u-boot runs the sound init code but did you try
> booting a signed FIT image using the verified u-boot?

Right, I'd bet that it's actually verified boot that's calling the
sound init code (probably just in case it needs to beep at the user).
...so this would happen even without chain booting.  I haven't tried
signing / booting without chain booting, but I can totally believe it
would hang just as easily.


> I'm asking because I'm see the hang both when chain-booting nv-u-boot from a
> kernel partition and booting a signed FIT image with the verified u-boot.
>
> So it seems this is a regression in latest U-boot and I didn't run into this
> before due having an older RW U-Boot on my Peach pit?

You must have had a very old RW U-Boot, possibly one before audio
support was added.  I think we just need to fix this in the kernel so
it doesn't hang.

-Doug
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Doug Anderson]

Tomasz,

On Tue, Jun 10, 2014 at 12:59 PM, Tomasz Figa  wrote:
> On 10.06.2014 21:58, Doug Anderson wrote:
>> Tomasz,
>>
>> On Tue, Jun 10, 2014 at 12:49 PM, Tomasz Figa  wrote:
>>> This is wrong. The "cs-gpios" property is supposed to be an array,
>>> indexed by chip select number of SPI devices (indicated by their "reg"
>>> properties).
>>>
>>> Moreover, is there a need to parse this manually in this driver? I can
>>> see respective parsing code in of_spi_register_master().
>>
>> I noticed this too (see my response), but I was confused about the
>> fact that nobody else uses the array created by
>> of_spi_register_master().  Any idea why?
>
> Hmm, I can see of_spi_register_master() assigning allocated pointer to
> master->cs_gpios, which is then used in spi_add_device() as follows:
>
> if (master->cs_gpios)
> spi->cs_gpio = master->cs_gpios[spi->chip_select];

OK.  I haven't traced through all the code, but I did notice:

* spi-gpio.c: has its own cs_gpios[0], initted with of_get_named_gpio()
* spi-dw-mmio.c, spi-efm32.c, spi-imx.c, spi-pl022.c, spi-sirf.c all
seem to be looking at "cs-gpios" directly.

...but I see now that you're right that most drivers don't need to
actually look at cs_gpios and can just look at the device itself.

-Doug
--
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: exynos5420-peach-pi: linux-next boot fails unless mau_epll left enabled?

[Javier Martinez Canillas]

Hello Doug,

On 06/10/2014 07:39 PM, Doug Anderson wrote:
> Hi,
> 
> On Mon, Jun 9, 2014 at 11:48 PM, Shaik Ameer Basha
>  wrote:
>> Hi Kevin,
>>
>> We tested on 3 "peach-pi" boards. We are not observing this issue.
>>
>> Even I tried with the below defconfig mentioned by you. No issues observed.
>> https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/eclass/cros-kernel/exynos5_defconfig
>>
>> This is the u-boot version currently we are using.
>> U-Boot 2013.04 (Feb 13 2014 - 16:35:03) for Peach
>>
>> Can you provide us more inputs like uboot version or any extra patches
>> you applied?
>> Also try removing all power domain nodes from exynos5420.dtsi and
>> check whether it is reproduced.
> 
> OK, we've narrowed it down so you can reproduce this and debug it...
> 
> To reproduce, simply run "sound init" at the U-Boot command prompt
> before booting the kernel.  ...or, alternatively, run "mw.l 0381
> 1".
> 
> Can you please confirm and then send up some patches to fix this?
> 
> --
> 
> I'm still a little confused about how Javier didn't run into this.
> Both Kevin and Javier are (I think) chain-booting nv-U-Boot from a
> kernel partition.  It appears that chain-booting U-Boot somehow runs
> the sound init code (at least for me).
>

Yes, I did not have this issue before. However... I installed the latest Peach
pit recovery image you provided me and mainline kernel started to hang on boot.
I remembered this thread so tested Kevin's patch which make it boot again.

You said that chain-booting u-boot runs the sound init code but did you try
booting a signed FIT image using the verified u-boot?

I'm asking because I'm see the hang both when chain-booting nv-u-boot from a
kernel partition and booting a signed FIT image with the verified u-boot.

So it seems this is a regression in latest U-boot and I didn't run into this
before due having an older RW U-Boot on my Peach pit?

> -Doug
> 

Best regards,
Javier
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Tomasz Figa]

On 10.06.2014 21:58, Doug Anderson wrote:
> Tomasz,
> 
> On Tue, Jun 10, 2014 at 12:49 PM, Tomasz Figa  wrote:
>> This is wrong. The "cs-gpios" property is supposed to be an array,
>> indexed by chip select number of SPI devices (indicated by their "reg"
>> properties).
>>
>> Moreover, is there a need to parse this manually in this driver? I can
>> see respective parsing code in of_spi_register_master().
> 
> I noticed this too (see my response), but I was confused about the
> fact that nobody else uses the array created by
> of_spi_register_master().  Any idea why?

Hmm, I can see of_spi_register_master() assigning allocated pointer to
master->cs_gpios, which is then used in spi_add_device() as follows:

if (master->cs_gpios)
spi->cs_gpio = master->cs_gpios[spi->chip_select];

Best regards,
Tomasz
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Doug Anderson]

Tomasz,

On Tue, Jun 10, 2014 at 12:49 PM, Tomasz Figa  wrote:
> This is wrong. The "cs-gpios" property is supposed to be an array,
> indexed by chip select number of SPI devices (indicated by their "reg"
> properties).
>
> Moreover, is there a need to parse this manually in this driver? I can
> see respective parsing code in of_spi_register_master().

I noticed this too (see my response), but I was confused about the
fact that nobody else uses the array created by
of_spi_register_master().  Any idea why?

-Doug
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Tomasz Figa]

Hi Naveen,

On 10.06.2014 12:08, Naveen Krishna Chatradhi wrote:
> Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
> defined under "controller-data" node under each slave node.

[snip]

> @@ -85,6 +83,7 @@ Example:
>   #size-cells = <0>;
>   pinctrl-names = "default";
>   pinctrl-0 = <&spi0_bus>;
> + cs-gpios = <&gpa2 5 1 0 3>;

While at it, you might also update the example to something more
appropriate on Samsung platforms, e.g. <&gpa2 5 0>;

>  
>   w25q80bw@0 {
>   #address-cells = <1>;

[snip]

> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
> *spi)
> +{
> + struct device_node *parent_np = NULL;
> + struct s3c64xx_spi_driver_data *sdd;
> + struct s3c64xx_spi_csinfo *cs;
> +
> + parent_np = of_get_parent(spi->dev.of_node);
> + if (!parent_np) {
> + dev_err(&spi->dev, "Parent node not found\n");
>   return ERR_PTR(-EINVAL);
>   }
>  
> + sdd = spi_master_get_devdata(spi->master);
> +
>   cs = kzalloc(sizeof(*cs), GFP_KERNEL);
>   if (!cs) {
> - of_node_put(data_np);
> + of_node_put(parent_np);
>   return ERR_PTR(-ENOMEM);
>   }
>  
>   /* The CS line is asserted/deasserted by the gpio pin */
>   if (sdd->cs_gpio)
> - cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
> + cs->line = of_get_named_gpio(parent_np, "cs-gpios", 0);

This is wrong. The "cs-gpios" property is supposed to be an array,
indexed by chip select number of SPI devices (indicated by their "reg"
properties).

Moreover, is there a need to parse this manually in this driver? I can
see respective parsing code in of_spi_register_master().

Best regards,
Tomasz
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Tomasz Figa]

On 10.06.2014 20:09, Doug Anderson wrote:
> Naveen / Sylwester,
> 
> On Tue, Jun 10, 2014 at 4:00 AM, Naveen Krishna Ch
>  wrote:
>>> Can we support both "cs-gpio" and "cs-gpios" for backward compatibility ?
>>> After your change all DTBs using the original pattern will not work with
>>> new kernels any more. At least I would expect such backward compatibility
>>> maintained for few kernel releases.
>>
>> The reason behind removing the "cs-gpio" or not providing backward
>> compatibility was
>>
>> 1. Since spi-core started using "cs-gpios" string from spi device node
>> several months ago.
>>   The spi-s3c64xx.c driver is partially broken for more than 6 months.
>>
>> 2. Supporting "cs-gpio" would add extra bit of code.
>>
>> I've corrected the dts files that were using "cs-gpio" under
>> "controller-data"(child node)
>> to use "cs-gpio" from spi device node (parent node).
>>
>> I will make another version if you insist.
> 
> Yup, as near as I can tell this has been broken since (3146bee spi:
> s3c64xx: Added provision for dedicated cs pin).  That landed June 25th
> of 2013 into the SPI tree.
> 
> ...so clearly nobody was really testing Samsung's SPI driver on ToT
> Linux.  1 year of unnoticed brokenness seems like enough time that we
> could do away with the old code.  If someone comes out of the woodwork
> and claims a dire need to support the old binding then it can be added
> then.
> 
> In-tree users of this appear to be:
> 
> arch/arm/boot/dts/exynos4210-smdkv310.dts:
>  cs-gpio = <&gpc1 2 0>;
> arch/arm/boot/dts/exynos4412-trats2.dts:
>  cs-gpio = <&gpb 5 0>;
> arch/arm/boot/dts/exynos5250-smdk5250.dts:
>  cs-gpio = <&gpa2 5 0>;
> 
> ...and I guess nobody has bothered using the SPI flash on those boards
> for the last year?

On Trats2, it's actually a camera sensor, not SPI flash...

Still, that's really a shame that:

a) such patch went in (i.e. its brokenness was not spotted in review),
b) the regression was not spotted.

Anyway, IMHO this justifies removing the old binding then.

Best regards,
Tomasz
--
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: linux-next on Chromebook2: DRM failing to allocate

[Kevin Hilman]

On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
 wrote:
> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman  wrote:
>> I'm trying to get the latest linux-next working on my Chromebook2
>> (it's booting to a serial console) and am now trying to get the
>> display working (at least for a frambuffer console.)
>>
>> Since the display nodes seem to be present in the exynos5800-peach-pi
>> DTS, I tried enabling DRM and it's failing to allocate memory (log
>> below[1]
>>
>> Is there some additional memory setup/allocations I should be doing?
>> maybe with CMA?
>
> Probably not CMA, but maybe you don't have the iommu enabled?

Turns out it was missing CMA support.  Specifically:
   CONFIG_CMA=y
   CONFIG_DMA_CMA=y
are needed (my full .config is here: http://hastebin.com/uqopirazir.vbs)

With that, it allocates, appears to detect the panel and even claims
"Console: switching to colour frame buffer device", but I don't see
tux or any output on the display (DRM debug output below).

Note that I'm chain-loading nv_uboot from an SD card, and u-boot is
driving the display (black text on white background.)  As soon as it
starts the kernel though, u-boot seems to shut down the display
(though the backlight seems to still be on.)

Maybe the DT for peach-pi is missing the regulator used to power the
panel, or maybe a GPIO used to power up the panel?

Any ideas?

Kevin

[1] DRM output with drm.debug=0xff  (full kernel boot log here:
http://hastebin.com/xigofelepe.vhdl

[1.192850] [drm] Initialized drm 1.1.0 20060810
[1.197676] [drm:drm_platform_init]
[1.200224] [drm:drm_get_platform_dev]
[1.204092] [drm:drm_minor_register]
[1.207851] [drm:drm_minor_register] new minor assigned 64
[1.213154] [drm:drm_minor_register]
[1.216791] [drm:drm_minor_register]
[1.220608] [drm:drm_minor_register] new minor assigned 0
[1.225869] [drm:exynos_drm_encoder_create] possible_crtcs = 0x1
[1.231820] [drm:exynos_drm_encoder_create] encoder has been created
[1.238295] [drm:drm_sysfs_connector_add] adding "eDP-1" to sysfs
[1.244222] [drm:drm_sysfs_hotplug_event] generating hotplug event
[1.250371] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[1.256953] [drm] No driver support for vblank timestamp query.
[1.262855] [drm:drm_helper_hpd_irq_event] [CONNECTOR:15:eDP-1]
status updated from unknown to connected
[1.272309] [drm:drm_sysfs_hotplug_event] generating hotplug event
[1.278480] [drm:exynos_drm_encoder_dpms] encoder dpms: 3
[1.283849] [drm:exynos_drm_crtc_dpms] crtc[6] mode[3]
[1.288964] [drm:exynos_drm_crtc_dpms] desired dpms mode is same as
previous one.
[1.296428] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:15:eDP-1]
[1.304588] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:15:eDP-1] probed modes :
[1.314043] [drm:drm_mode_debug_printmodeline] Modeline
16:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
0x0
[1.325660] [drm:drm_setup_crtcs]
[1.329044] [drm:drm_enable_connectors] connector 15 enabled? yes
[1.335116] [drm:drm_target_preferred] looking for cmdline mode on
connector 15
[1.342402] [drm:drm_target_preferred] looking for preferred mode
on connector 15
[1.349862] [drm:drm_target_preferred] found mode 1920x1080
[1.355414] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
[1.361749] [drm:drm_setup_crtcs] desired mode 1920x1080 set on crtc 6
[1.368257] [drm:exynos_drm_fbdev_create] surface width(1920),
height(1080) and bpp(32
[1.376150] [drm:exynos_drm_init_buf] desired size = 0x7e9000
[1.381886] [drm:exynos_drm_gem_init] created file object = 0xec7e2000
[1.391732] [drm:lowlevel_buffer_allocate] dma_addr(0x8e90),
size(0x7e9000)
[1.397581] [drm:drm_framebuffer_reference] FB ID: 18
[1.402614] [drm:exynos_drm_fb_buffer] dma_addr = 0x8e90
[1.408526] [drm:drm_crtc_helper_set_config]
[1.408531] [drm:drm_crtc_helper_set_config] [CRTC:6] [FB:18]
#connectors=1 (x y) (0 0)
[1.408536] [drm:drm_crtc_helper_set_config] crtc has no fb, full mode set
[1.408539] [drm:drm_crtc_helper_set_config] modes are different,
full mode set
[1.408544] [drm:drm_mode_debug_printmodeline] Modeline 0:"" 0 0 0
0 0 0 0 0 0 0 0x0 0x0
[1.408549] [drm:drm_mode_debug_printmodeline] Modeline
17:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
0x0
[1.408553] [drm:drm_crtc_helper_set_config] encoder changed, full
mode switch
[1.408556] [drm:drm_crtc_helper_set_config] crtc changed, full mode switch
[1.408560] [drm:drm_crtc_helper_set_config] [CONNECTOR:15:eDP-1] to [CRTC:6]
[1.408563] [drm:drm_crtc_helper_set_config] attempting to set mode
from userspace
[1.408568] [drm:drm_mode_debug_printmodeline] Modeline
17:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
0x0
[1.408574] [drm:drm_crtc_helper_set_mode] [CRTC:6]
[1.408579] [drm:exynos_drm_fb_buffer] dma_addr =

Re: [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Tomasz Figa]

On 10.06.2014 20:26, Doug Anderson wrote:
> Naveen,
> 
> Not a full review, but a few quick things I happened to notice:
> 
> On Tue, Jun 10, 2014 at 3:08 AM, Naveen Krishna Chatradhi
>  wrote:
>> @@ -94,7 +93,6 @@ Example:
>> spi-max-frequency = <1>;
>>
>> controller-data {
>> -   cs-gpio = <&gpa2 5 1 0 3>;
>> samsung,spi-feedback-delay = <0>;
> 
> In a future patch (not this one!) it might make sense to also move
> spi-feedback-delay out.
> 
> 
>> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
>> *spi)
> 
> I believe that you can make use of the fact that the SPI core already
> got these GPIOs for you.  See of_spi_register_master().  Everything
> you need ought to be in master->num_chipselect and master->cs_gpios.
> 
> Strangely, it doesn't look like any other drivers use this, so
> possibly I'm confused...
> 
> 
>> @@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>> struct s3c64xx_spi_info *sci;
>> int err;
>>
>> +   if (!spi->dev.of_node) {
>> +   dev_err(&spi->dev, "device node not found\n");
>> +   return -EINVAL;
>> +   }
> 
> This seems incredibly broken.  You're saying that the SPI driver no
> longer works without the device tree?  I don't think that's desirable,
> but I suppose I could be wrong.
> 
> Also note that you still left an "if" test below for trying to deal
> with the fact that there is no "of_node".

This driver can be also used by non-DT platforms (namely s3c2443,
s3c64xx and s5p*) and so this assumption is wrong.

Best regards,
Tomasz
--
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: Problems booting exynos5420 with >1 CPU

[Nicolas Pitre]

On Tue, 10 Jun 2014, Catalin Marinas wrote:
> On Tue, Jun 10, 2014 at 05:49:01PM +0100, Nicolas Pitre wrote:
> > The M-class processor should be treated the same way as firmware.  It 
> > ought to be flexible (certainly more than hardwired hardware), but it 
> > shares all the same downsides as firmware and the same concerns apply.
> 
> Yes, we can treat it as firmware, but we don't have a better alternative
> to move the functionality into the kernel (well, we could at least allow
> the kernel to load a binary blob and restart the controller).

That would address the "easy to update in the field" side of the story.  
So far I've not seen this aspect been addressed with a serious plan 
anywhere.

> > The reaction from the hardware people often is "the software is crap and 
> > makes our hardware look bad, we know better, so let's make it easier on 
> > those poor software dudes by handling power management in hardware 
> > instead".  But ultimately the hardware just can't predict things like 
> > software can.  It might do a better job than the current software state 
> > of affairs, but most likely not be as efficient as a proper software 
> > architecture.  The hardware may only be reactive, whereas the software 
> > can be proactive (when properly done that is).
> 
> Indeed. But that's not the aim of the power controller on our boards.
> It's just a mechanism for safely changing sleep states, CPU frequencies
> but entirely under the OS decision.

Sure.  But then you might want to consider that some usage scenarios 
might benefit from the ability to abort a request, or monitor the 
progress of a request for software timing purposes, or accept parallel 
requests rather than serialize them, etc.  Given the flexibility to 
extend beyond a rigid interface, the system may become even more 
efficient overall, albeit with added complexity in the implementation. 
But for that to work it has to be cheaply achievable.

> > I sense from your paragraph above that ARM might be going the same 
> > direction as X86 and that would be very sad.  Maybe the best compromise 
> > would be for all knobs to be made available to software if software 
> > wants to turn off the hardware auto-pilot and take control.  This way 
> > the hardware guys would cover their arses while still allowing for the 
> > possibility that software might be able to out perform the hardware 
> > solution.
> 
> I'm definitely not suggesting ARM is going the same route. Just trying
> to show that ARM is slightly better here.
> 
> As a personal opinion, I like the simplicity of writing a register to
> change the P-state but I don't like the non-determinism of the x86
> hardware w.r.t. CPU performance. There are however some "policy" aspects
> which I find interesting (like detecting whether the workload is memory
> bound and automatically lowering the CPU frequency; the OS cannot react
> this fast).

This is not really a policy.  This is a straight-forward waterfall 
dependency.  There is simply nothing you can do with those CPU clock 
cycles when stalled most of the time waiting for memory queries to come 
back so the choice is obvious.  If however this has a significant impact 
on code execution speed then this becomes a tradeoff and the choice to 
use this feature or not (the policy) must be implemented in software.


Nicolas
--
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: linux-next on Chromebook2: DRM failing to allocate

[Tomasz Figa]

On 10.06.2014 20:04, Stéphane Marchesin wrote:
> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman  wrote:
>> I'm trying to get the latest linux-next working on my Chromebook2
>> (it's booting to a serial console) and am now trying to get the
>> display working (at least for a frambuffer console.)
>>
>> Since the display nodes seem to be present in the exynos5800-peach-pi
>> DTS, I tried enabling DRM and it's failing to allocate memory (log
>> below[1]
>>
>> Is there some additional memory setup/allocations I should be doing?
>> maybe with CMA?
> 
> Probably not CMA, but maybe you don't have the iommu enabled?
> 

It should work without IOMMU as well. We're using Exynos DRM on Exynos4
without IOMMU and with CMA instead and it works fine.

Best regards,
Tomasz
--
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: linux-next on Chromebook2: DRM failing to allocate

[Kevin Hilman]

On Tue, Jun 10, 2014 at 11:24 AM, Kevin Hilman  wrote:
> On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
>  wrote:
>> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman  wrote:
>>> I'm trying to get the latest linux-next working on my Chromebook2
>>> (it's booting to a serial console) and am now trying to get the
>>> display working (at least for a frambuffer console.)
>>>
>>> Since the display nodes seem to be present in the exynos5800-peach-pi
>>> DTS, I tried enabling DRM and it's failing to allocate memory (log
>>> below[1]
>>>
>>> Is there some additional memory setup/allocations I should be doing?
>>> maybe with CMA?
>>
>> Probably not CMA, but maybe you don't have the iommu enabled?
>
> Thanks!  I didn'th ave that enabled in the .config.
> With that, it seems to allocate, but fails to probe:

Oops, ignore this one... operator error.
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Doug Anderson]

Naveen,

Not a full review, but a few quick things I happened to notice:

On Tue, Jun 10, 2014 at 3:08 AM, Naveen Krishna Chatradhi
 wrote:
> @@ -94,7 +93,6 @@ Example:
> spi-max-frequency = <1>;
>
> controller-data {
> -   cs-gpio = <&gpa2 5 1 0 3>;
> samsung,spi-feedback-delay = <0>;

In a future patch (not this one!) it might make sense to also move
spi-feedback-delay out.


> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
> *spi)

I believe that you can make use of the fact that the SPI core already
got these GPIOs for you.  See of_spi_register_master().  Everything
you need ought to be in master->num_chipselect and master->cs_gpios.

Strangely, it doesn't look like any other drivers use this, so
possibly I'm confused...


> @@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
> struct s3c64xx_spi_info *sci;
> int err;
>
> +   if (!spi->dev.of_node) {
> +   dev_err(&spi->dev, "device node not found\n");
> +   return -EINVAL;
> +   }

This seems incredibly broken.  You're saying that the SPI driver no
longer works without the device tree?  I don't think that's desirable,
but I suppose I could be wrong.

Also note that you still left an "if" test below for trying to deal
with the fact that there is no "of_node".


-Doug
--
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: linux-next on Chromebook2: DRM failing to allocate

[Kevin Hilman]

On Tue, Jun 10, 2014 at 11:04 AM, Stéphane Marchesin
 wrote:
> On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman  wrote:
>> I'm trying to get the latest linux-next working on my Chromebook2
>> (it's booting to a serial console) and am now trying to get the
>> display working (at least for a frambuffer console.)
>>
>> Since the display nodes seem to be present in the exynos5800-peach-pi
>> DTS, I tried enabling DRM and it's failing to allocate memory (log
>> below[1]
>>
>> Is there some additional memory setup/allocations I should be doing?
>> maybe with CMA?
>
> Probably not CMA, but maybe you don't have the iommu enabled?

Thanks!  I didn'th ave that enabled in the .config.
With that, it seems to allocate, but fails to probe:

[0.829231] [drm] Initialized drm 1.1.0 20060810
[0.833153] [drm:drm_platform_init]
[0.835947] [drm:drm_get_platform_dev]
[0.839813] [drm:drm_minor_register]
[0.843580] [drm:drm_minor_register] new minor assigned 64
[0.848877] [drm:drm_minor_register]
[0.852512] [drm:drm_minor_register]
[0.856319] [drm:drm_minor_register] new minor assigned 0
[0.861568] [drm:exynos_drm_create_enc_conn] *ERROR* failed to create encoder
[0.868651] [drm:exynos_drm_initialize_displays] *ERROR* Encoder
create [1] failed with -14
[0.877420] exynos-drm: probe of exynos-drm failed with error -14
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Doug Anderson]

Naveen / Sylwester,

On Tue, Jun 10, 2014 at 4:00 AM, Naveen Krishna Ch
 wrote:
>> Can we support both "cs-gpio" and "cs-gpios" for backward compatibility ?
>> After your change all DTBs using the original pattern will not work with
>> new kernels any more. At least I would expect such backward compatibility
>> maintained for few kernel releases.
>
> The reason behind removing the "cs-gpio" or not providing backward
> compatibility was
>
> 1. Since spi-core started using "cs-gpios" string from spi device node
> several months ago.
>   The spi-s3c64xx.c driver is partially broken for more than 6 months.
>
> 2. Supporting "cs-gpio" would add extra bit of code.
>
> I've corrected the dts files that were using "cs-gpio" under
> "controller-data"(child node)
> to use "cs-gpio" from spi device node (parent node).
>
> I will make another version if you insist.

Yup, as near as I can tell this has been broken since (3146bee spi:
s3c64xx: Added provision for dedicated cs pin).  That landed June 25th
of 2013 into the SPI tree.

...so clearly nobody was really testing Samsung's SPI driver on ToT
Linux.  1 year of unnoticed brokenness seems like enough time that we
could do away with the old code.  If someone comes out of the woodwork
and claims a dire need to support the old binding then it can be added
then.

In-tree users of this appear to be:

arch/arm/boot/dts/exynos4210-smdkv310.dts:
 cs-gpio = <&gpc1 2 0>;
arch/arm/boot/dts/exynos4412-trats2.dts:
 cs-gpio = <&gpb 5 0>;
arch/arm/boot/dts/exynos5250-smdk5250.dts:
 cs-gpio = <&gpa2 5 0>;

...and I guess nobody has bothered using the SPI flash on those boards
for the last year?
--
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: linux-next on Chromebook2: DRM failing to allocate

[Stéphane Marchesin]

On Tue, Jun 10, 2014 at 10:56 AM, Kevin Hilman  wrote:
> I'm trying to get the latest linux-next working on my Chromebook2
> (it's booting to a serial console) and am now trying to get the
> display working (at least for a frambuffer console.)
>
> Since the display nodes seem to be present in the exynos5800-peach-pi
> DTS, I tried enabling DRM and it's failing to allocate memory (log
> below[1]
>
> Is there some additional memory setup/allocations I should be doing?
> maybe with CMA?

Probably not CMA, but maybe you don't have the iommu enabled?

Stéphane

>
> Thanks for any advice,
>
> Kevin
>
>
> [1] DRM output with drm.debug=0xf on command line
>
> [1.095622] [drm] Initialized drm 1.1.0 20060810
> [1.099792] [drm:drm_platform_init]
> [1.102311] [drm:drm_get_platform_dev]
> [1.106211] [drm:drm_minor_register]
> [1.11] [drm:drm_minor_register] new minor assigned 64
> [1.115265] [drm:drm_minor_register]
> [1.118900] [drm:drm_minor_register]
> [1.122693] [drm:drm_minor_register] new minor assigned 0
> [1.127970] [drm:exynos_drm_encoder_create] possible_crtcs = 0x1
> [1.133915] [drm:exynos_drm_encoder_create] encoder has been created
> [1.140407] [drm:drm_sysfs_connector_add] adding "eDP-1" to sysfs
> [1.146335] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [1.152488] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [1.159065] [drm] No driver support for vblank timestamp query.
> [1.164965] [drm:drm_helper_hpd_irq_event] [CONNECTOR:15:eDP-1]
> status updated from unknown to connected
> [1.174418] [drm:drm_sysfs_hotplug_event] generating hotplug event
> [1.180590] [drm:exynos_drm_encoder_dpms] encoder dpms: 3
> [1.185959] [drm:exynos_drm_crtc_dpms] crtc[6] mode[3]
> [1.191074] [drm:exynos_drm_crtc_dpms] desired dpms mode is same as
> previous one.
> [1.198538] [drm:drm_helper_probe_single_connector_modes_merge_bits]
> [CONNECTOR:15:eDP-1]
> [1.206698] [drm:drm_helper_probe_single_connector_modes_merge_bits]
> [CONNECTOR:15:eDP-1] probed modes :
> [1.216149] [drm:drm_mode_debug_printmodeline] Modeline
> 16:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
> 0x0
> [1.227779] [drm:drm_setup_crtcs]
> [1.231135] [drm:drm_enable_connectors] connector 15 enabled? yes
> [1.237227] [drm:drm_target_preferred] looking for cmdline mode on
> connector 15
> [1.244512] [drm:drm_target_preferred] looking for preferred mode
> on connector 15
> [1.251972] [drm:drm_target_preferred] found mode 1920x1080
> [1.257525] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
> [1.263859] [drm:drm_setup_crtcs] desired mode 1920x1080 set on crtc 6
> [1.270367] [drm:exynos_drm_fbdev_create] surface width(1920),
> height(1080) and bpp(32
> [1.278260] [drm:exynos_drm_init_buf] desired size = 0x7e9000
> [1.283995] [drm:exynos_drm_gem_init] created file object = 0xed7f9200
> [1.290502] [ cut here ]
> [1.295094] WARNING: CPU: 1 PID: 1 at ../mm/page_alloc.c:2509
> __alloc_pages_nodemask+0x220/0x76c()
> [1.304022] Modules linked in:
> [1.307044] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
> 3.15.0-rc8-next-20140605-4-gde16f7d3b2b5-dirty #31
> [1.316882] [<800152ac>] (unwind_backtrace) from [<80011d94>]
> (show_stack+0x20/0x24)
> [1.324595] [<80011d94>] (show_stack) from [<805128d8>]
> (dump_stack+0x74/0x90)
> [1.331790] [<805128d8>] (dump_stack) from [<80024208>]
> (warn_slowpath_common+0x78/0x9c)
> [1.339854] [<80024208>] (warn_slowpath_common) from [<80024258>]
> (warn_slowpath_null+0x2c/0x34)
> [1.348622] [<80024258>] (warn_slowpath_null) from [<800c9334>]
> (__alloc_pages_nodemask+0x220/0x76c)
> [1.357730] [<800c9334>] (__alloc_pages_nodemask) from [<80019fb8>]
> (__dma_alloc_buffer.isra.13+0x3c/0x184)
> [1.367443] [<80019fb8>] (__dma_alloc_buffer.isra.13) from
> [<8001a124>] (__alloc_remap_buffer.isra.15+0x24/0xc0)
> [1.377592] [<8001a124>] (__alloc_remap_buffer.isra.15) from
> [<8001a410>] (__dma_alloc+0x250/0x2e0)
> [1.386613] [<8001a410>] (__dma_alloc) from [<8001a5d8>]
> (arm_dma_alloc+0x94/0xa0)
> [1.394166] [<8001a5d8>] (arm_dma_alloc) from [<802cf650>]
> (exynos_drm_alloc_buf+0x118/0x294)
> [1.402663] [<802cf650>] (exynos_drm_alloc_buf) from [<802cfd10>]
> (exynos_drm_gem_create+0x84/0xec)
> [1.411684] [<802cfd10>] (exynos_drm_gem_create) from [<802cea14>]
> (exynos_drm_fbdev_create+0xdc/0x2ec)
> [1.421056] [<802cea14>] (exynos_drm_fbdev_create) from
> [<802b306c>] (drm_fb_helper_initial_config+0x324/0x440)
> [1.431117] [<802b306c>] (drm_fb_helper_initial_config) from
> [<802ced58>] (exynos_drm_fbdev_init+0xd0/0x10c)
> [1.440918] [<802ced58>] (exynos_drm_fbdev_init) from [<802cef10>]
> (exynos_drm_output_poll_changed+0x34/0x38)
> [1.450808] [<802cef10>] (exynos_drm_output_poll_changed) from
> [<802b0a08>] (drm_kms_helper_hotplug_event+0x34/0x38)
> [1.

linux-next on Chromebook2: DRM failing to allocate

[Kevin Hilman]

I'm trying to get the latest linux-next working on my Chromebook2
(it's booting to a serial console) and am now trying to get the
display working (at least for a frambuffer console.)

Since the display nodes seem to be present in the exynos5800-peach-pi
DTS, I tried enabling DRM and it's failing to allocate memory (log
below[1]

Is there some additional memory setup/allocations I should be doing?
maybe with CMA?

Thanks for any advice,

Kevin


[1] DRM output with drm.debug=0xf on command line

[1.095622] [drm] Initialized drm 1.1.0 20060810
[1.099792] [drm:drm_platform_init]
[1.102311] [drm:drm_get_platform_dev]
[1.106211] [drm:drm_minor_register]
[1.11] [drm:drm_minor_register] new minor assigned 64
[1.115265] [drm:drm_minor_register]
[1.118900] [drm:drm_minor_register]
[1.122693] [drm:drm_minor_register] new minor assigned 0
[1.127970] [drm:exynos_drm_encoder_create] possible_crtcs = 0x1
[1.133915] [drm:exynos_drm_encoder_create] encoder has been created
[1.140407] [drm:drm_sysfs_connector_add] adding "eDP-1" to sysfs
[1.146335] [drm:drm_sysfs_hotplug_event] generating hotplug event
[1.152488] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[1.159065] [drm] No driver support for vblank timestamp query.
[1.164965] [drm:drm_helper_hpd_irq_event] [CONNECTOR:15:eDP-1]
status updated from unknown to connected
[1.174418] [drm:drm_sysfs_hotplug_event] generating hotplug event
[1.180590] [drm:exynos_drm_encoder_dpms] encoder dpms: 3
[1.185959] [drm:exynos_drm_crtc_dpms] crtc[6] mode[3]
[1.191074] [drm:exynos_drm_crtc_dpms] desired dpms mode is same as
previous one.
[1.198538] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:15:eDP-1]
[1.206698] [drm:drm_helper_probe_single_connector_modes_merge_bits]
[CONNECTOR:15:eDP-1] probed modes :
[1.216149] [drm:drm_mode_debug_printmodeline] Modeline
16:"1920x1080" 60 150660 1920 1980 2060 2232 1080 1090 1100 1125 0x48
0x0
[1.227779] [drm:drm_setup_crtcs]
[1.231135] [drm:drm_enable_connectors] connector 15 enabled? yes
[1.237227] [drm:drm_target_preferred] looking for cmdline mode on
connector 15
[1.244512] [drm:drm_target_preferred] looking for preferred mode
on connector 15
[1.251972] [drm:drm_target_preferred] found mode 1920x1080
[1.257525] [drm:drm_setup_crtcs] picking CRTCs for 4096x4096 config
[1.263859] [drm:drm_setup_crtcs] desired mode 1920x1080 set on crtc 6
[1.270367] [drm:exynos_drm_fbdev_create] surface width(1920),
height(1080) and bpp(32
[1.278260] [drm:exynos_drm_init_buf] desired size = 0x7e9000
[1.283995] [drm:exynos_drm_gem_init] created file object = 0xed7f9200
[1.290502] [ cut here ]
[1.295094] WARNING: CPU: 1 PID: 1 at ../mm/page_alloc.c:2509
__alloc_pages_nodemask+0x220/0x76c()
[1.304022] Modules linked in:
[1.307044] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
3.15.0-rc8-next-20140605-4-gde16f7d3b2b5-dirty #31
[1.316882] [<800152ac>] (unwind_backtrace) from [<80011d94>]
(show_stack+0x20/0x24)
[1.324595] [<80011d94>] (show_stack) from [<805128d8>]
(dump_stack+0x74/0x90)
[1.331790] [<805128d8>] (dump_stack) from [<80024208>]
(warn_slowpath_common+0x78/0x9c)
[1.339854] [<80024208>] (warn_slowpath_common) from [<80024258>]
(warn_slowpath_null+0x2c/0x34)
[1.348622] [<80024258>] (warn_slowpath_null) from [<800c9334>]
(__alloc_pages_nodemask+0x220/0x76c)
[1.357730] [<800c9334>] (__alloc_pages_nodemask) from [<80019fb8>]
(__dma_alloc_buffer.isra.13+0x3c/0x184)
[1.367443] [<80019fb8>] (__dma_alloc_buffer.isra.13) from
[<8001a124>] (__alloc_remap_buffer.isra.15+0x24/0xc0)
[1.377592] [<8001a124>] (__alloc_remap_buffer.isra.15) from
[<8001a410>] (__dma_alloc+0x250/0x2e0)
[1.386613] [<8001a410>] (__dma_alloc) from [<8001a5d8>]
(arm_dma_alloc+0x94/0xa0)
[1.394166] [<8001a5d8>] (arm_dma_alloc) from [<802cf650>]
(exynos_drm_alloc_buf+0x118/0x294)
[1.402663] [<802cf650>] (exynos_drm_alloc_buf) from [<802cfd10>]
(exynos_drm_gem_create+0x84/0xec)
[1.411684] [<802cfd10>] (exynos_drm_gem_create) from [<802cea14>]
(exynos_drm_fbdev_create+0xdc/0x2ec)
[1.421056] [<802cea14>] (exynos_drm_fbdev_create) from
[<802b306c>] (drm_fb_helper_initial_config+0x324/0x440)
[1.431117] [<802b306c>] (drm_fb_helper_initial_config) from
[<802ced58>] (exynos_drm_fbdev_init+0xd0/0x10c)
[1.440918] [<802ced58>] (exynos_drm_fbdev_init) from [<802cef10>]
(exynos_drm_output_poll_changed+0x34/0x38)
[1.450808] [<802cef10>] (exynos_drm_output_poll_changed) from
[<802b0a08>] (drm_kms_helper_hotplug_event+0x34/0x38)
[1.461305] [<802b0a08>] (drm_kms_helper_hotplug_event) from
[<802b1148>] (drm_helper_hpd_irq_event+0x108/0x120)
[1.471455] [<802b1148>] (drm_helper_hpd_irq_event) from
[<802cd998>] (exynos_drm_load+0xf0/0x130)
[1.480390] [<802cd998>] (exynos_drm_load) from [<802bb654>]
(drm_dev_register+0x90/0x110)
[1.488631

Re: Problems booting exynos5420 with >1 CPU

[Catalin Marinas]

On Tue, Jun 10, 2014 at 05:49:01PM +0100, Nicolas Pitre wrote:
> On Tue, 10 Jun 2014, Catalin Marinas wrote:
> > On Tue, Jun 10, 2014 at 12:25:47AM -0400, Nicolas Pitre wrote:
> > > On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote:
> > > > 4) When I am talking about firmware I am talking about sequences that
> > > >are very close to HW (disabling C bit, cleaning caches, exiting
> > > >coherency). Erratas notwithstanding, they are being standardized at
> > > >ARM the best we can. They might even end up being implemented in HW
> > > >in the not so far future. I understand they are tricky, I understand
> > > >they take lots of time to implement them and to debug them, what I
> > > >want to say is that they are becoming standard and we _must_ reuse 
> > > > the
> > > >same code for all ARM platforms. You can implement them in MCPM (see
> > > >(1)) or in firmware (and please do not start painting me as firmware
> > > >hugger here, I am referring to standard power down sequences that
> > > >again, are very close to HW state machines 
> > > 
> > > That's where the disconnect lies.  On the one hand you say "I understand 
> > > they are tricky, I understand they take lots of time to implement them 
> > > and to debug them" and on the other hand you say "They might end up being 
> > > implemented in HW in the not so far future."  That simply makes no 
> > > economical sense at all!
> > 
> > It makes lots of sense, though not from a software maintainability
> > perspective. It would be nice if everything still looked like ARM7TDMI
> > but in the race for performance (vs power), hardware becomes more
> > complex and it's not just the CPU but adjacent parts like interconnects,
> > caches, asynchronous bridges, voltage shifters, memory controllers,
> > clocks/PLLs etc. Many of these are simply hidden from the high level OS
> > like Linux because the OS assumes certain configuration (e.g. access to
> > memory) and it's only the hardware itself that knows in what order they
> > can be turned on or off (when triggered explicitly by the OS or an
> > external event).
> 
> I agree when the hardware has to handle parallel dependencies ordered in 
> waterfall style. In such cases there is usually no point relying on 
> software to implement what is nevertheless simple determinism with 
> no possible alternative usage.
> 
> But the *most* important thing is what you put in parens, so let me 
> emphasize on what you just said:
> 
>   When triggered _explicitly_ by the OS or external events

I don't think anyone is arguing that the policy should not be in the OS.
But part of the mechanism can be in the OS and part in firmware, SCP or
hardware. The kernel part can be a simple PSCI call or more complex
setup (possibly MCPM-based) which usually ends up with a WFI. This WFI,
however, triggers further hardware changes which may be handled by
dedicated power controller.

> > Having an dedicated power controller (e.g. M-class
> > processor) to handle some of these is a rather flexible approach, other
> > bits require RTL (and usually impossible to update).
> 
> The M-class processor should be treated the same way as firmware.  It 
> ought to be flexible (certainly more than hardwired hardware), but it 
> shares all the same downsides as firmware and the same concerns apply.

Yes, we can treat it as firmware, but we don't have a better alternative
to move the functionality into the kernel (well, we could at least allow
the kernel to load a binary blob and restart the controller).

> > > When some operation is 1) tricky and takes time to debug, and 2) not 
> > > performance critical (no one is trying to get in and out of idle or 
> > > hibernation a billion times per second), then you should never ever put 
> > > such a thing in firmware, and hardware should be completely out of the 
> > > question!
> > 
> > I agree that things can go wrong (both in hardware and software, no
> > matter where it runs) but please don't think that such power
> > architecture has been specifically engineered to hide the hardware from
> > Linux. It's a necessity for complex systems and the optimal solution is
> > not always simplification (it's not just ARM+vendors doing this, just
> > look at the power model of modern x86 processors, hidden nicely from the
> > software behind a few registers while making things harder for scheduler
> > which cannot rely on a constant performance level; but it's a trade-off
> > they are happy to make).
> 
> I'll claim that this is a bad tradeoff.  And the reason why some 
> hardware architects might think it is a good one is because so far we 
> really sucked at software based power management in Linux (and possibly 
> other OSes as well).  Hence the (fairly recent) realization that power 
> management has to be integrated and under control of the scheduler 
> rather than existing as some ad hoc subsystem.

But even this is a complex problem. Anyway, I don't think the (ARM at
least) hardwa

Re: exynos5420-peach-pi: linux-next boot fails unless mau_epll left enabled?

[Doug Anderson]

Hi,

On Mon, Jun 9, 2014 at 11:48 PM, Shaik Ameer Basha
 wrote:
> Hi Kevin,
>
> We tested on 3 "peach-pi" boards. We are not observing this issue.
>
> Even I tried with the below defconfig mentioned by you. No issues observed.
> https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/eclass/cros-kernel/exynos5_defconfig
>
> This is the u-boot version currently we are using.
> U-Boot 2013.04 (Feb 13 2014 - 16:35:03) for Peach
>
> Can you provide us more inputs like uboot version or any extra patches
> you applied?
> Also try removing all power domain nodes from exynos5420.dtsi and
> check whether it is reproduced.

OK, we've narrowed it down so you can reproduce this and debug it...

To reproduce, simply run "sound init" at the U-Boot command prompt
before booting the kernel.  ...or, alternatively, run "mw.l 0381
1".

Can you please confirm and then send up some patches to fix this?

--

I'm still a little confused about how Javier didn't run into this.
Both Kevin and Javier are (I think) chain-booting nv-U-Boot from a
kernel partition.  It appears that chain-booting U-Boot somehow runs
the sound init code (at least for me).

-Doug
--
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 v4 06/11] ARM: EXYNOS: Add support for mapping PMU base address via DT

[Tomasz Figa]

Hi,

On 10.05.2014 08:56, Pankaj Dubey wrote:
> From: Young-Gun Jang 
> 
> Add support for mapping Samsung Power Management Unit (PMU)
> base address from device tree. This patch also adds helper
> function as "get_exynos_pmuregmap". This function can be used
> by other machine files such as "pm.c", "hotplug.c" for accessing
> PMU regmap handle.
> 

I don't think there is a need to use regmap to provide access to PMU to
such low level code such as pm.c or hotplug.c. Moreover, I believe that
it might be undesirable in some cases, e.g. very low level code running
at early resume or late suspend.

IMHO, based on what we now have for SYSRAM, you could simply map PMU
from device tree one time, before SMP init, and keep the address in some
globally accessible variable, like those for SYSRAM we have right now
(sysram_base_addr, sysram_ns_base_addr -> pmu_base_addr).

Then, registration of the normal syscon would happen through standard
platform driver mechanisms and no special early handling would be necessary.

Best regards,
Tomasz
--
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: Problems booting exynos5420 with >1 CPU

[Nicolas Pitre]

On Tue, 10 Jun 2014, Catalin Marinas wrote:

> Hi Nico,
> 
> Sorry, I can't stay away from this thread ;)

;-)

> On Tue, Jun 10, 2014 at 12:25:47AM -0400, Nicolas Pitre wrote:
> > On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote:
> > > 4) When I am talking about firmware I am talking about sequences that
> > >are very close to HW (disabling C bit, cleaning caches, exiting
> > >coherency). Erratas notwithstanding, they are being standardized at
> > >ARM the best we can. They might even end up being implemented in HW
> > >in the not so far future. I understand they are tricky, I understand
> > >they take lots of time to implement them and to debug them, what I
> > >want to say is that they are becoming standard and we _must_ reuse the
> > >same code for all ARM platforms. You can implement them in MCPM (see
> > >(1)) or in firmware (and please do not start painting me as firmware
> > >hugger here, I am referring to standard power down sequences that
> > >again, are very close to HW state machines 
> > 
> > That's where the disconnect lies.  On the one hand you say "I understand 
> > they are tricky, I understand they take lots of time to implement them 
> > and to debug them" and on the other hand you say "They might end up being 
> > implemented in HW in the not so far future."  That simply makes no 
> > economical sense at all!
> 
> It makes lots of sense, though not from a software maintainability
> perspective. It would be nice if everything still looked like ARM7TDMI
> but in the race for performance (vs power), hardware becomes more
> complex and it's not just the CPU but adjacent parts like interconnects,
> caches, asynchronous bridges, voltage shifters, memory controllers,
> clocks/PLLs etc. Many of these are simply hidden from the high level OS
> like Linux because the OS assumes certain configuration (e.g. access to
> memory) and it's only the hardware itself that knows in what order they
> can be turned on or off (when triggered explicitly by the OS or an
> external event).

I agree when the hardware has to handle parallel dependencies ordered in 
waterfall style. In such cases there is usually no point relying on 
software to implement what is nevertheless simple determinism with 
no possible alternative usage.

But the *most* important thing is what you put in parents, so let me 
emphasize on what you just said:

When triggered _explicitly_ by the OS or external events

> Having an dedicated power controller (e.g. M-class
> processor) to handle some of these is a rather flexible approach, other
> bits require RTL (and usually impossible to update).

The M-class processor should be treated the same way as firmware.  It 
ought to be flexible (certainly more than hardwired hardware), but it 
shares all the same downsides as firmware and the same concerns apply.

> > When some operation is 1) tricky and takes time to debug, and 2) not 
> > performance critical (no one is trying to get in and out of idle or 
> > hibernation a billion times per second), then you should never ever put 
> > such a thing in firmware, and hardware should be completely out of the 
> > question!
> 
> I agree that things can go wrong (both in hardware and software, no
> matter where it runs) but please don't think that such power
> architecture has been specifically engineered to hide the hardware from
> Linux. It's a necessity for complex systems and the optimal solution is
> not always simplification (it's not just ARM+vendors doing this, just
> look at the power model of modern x86 processors, hidden nicely from the
> software behind a few registers while making things harder for scheduler
> which cannot rely on a constant performance level; but it's a trade-off
> they are happy to make).

I'll claim that this is a bad tradeoff.  And the reason why some 
hardware architects might think it is a good one is because so far we 
really sucked at software based power management in Linux (and possibly 
other OSes as well).  Hence the (fairly recent) realization that power 
management has to be integrated and under control of the scheduler 
rather than existing as some ad hoc subsystem.

The reaction from the hardware people often is "the software is crap and 
makes our hardware look bad, we know better, so let's make it easier on 
those poor software dudes by handling power management in hardware 
instead".  But ultimately the hardware just can't predict things like 
software can.  It might do a better job than the current software state 
of affairs, but most likely not be as efficient as a proper software 
architecture.  The hardware may only be reactive, whereas the software 
can be proactive (when properly done that is).

I sense from your paragraph above that ARM might be going the same 
direction as X86 and that would be very sad.  Maybe the best compromise 
would be for all knobs to be made available to software if software 
wants to turn off the hardware auto-pilot and take cont

Re: [PATCH] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start

[Nicolas Pitre]

On Tue, 10 Jun 2014, Doug Anderson wrote:

> My S-state knowledge is not strong, but I believe that Lorenzo's
> questions matter if we're using S2 for CPUidle (where we actually turn
> off power and hot unplug CPUs) but not when we're using S1 for CPUidle
> (where we just enter WFI/WFE).
> 
> I believe that in ChromeOS we use S1 CPUidle and that it works fine.
> We've never implemented S2 that I'm aware of.

You'll have to rely on MCPM for that.  That's probably why it hasn't 
been implemented before.


Nicolas
--
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 2/3] clk: exynos5420: Add IDs for clocks used in PD mfc

[Tomasz Figa]

Hi,

On 26.05.2014 13:56, Shaik Ameer Basha wrote:
> From: Arun Kumar K 
> 
> Adds IDs for MUX clocks to be used by power domain for MFC
> for doing re-parenting while pd on/off.
> 
> Signed-off-by: Arun Kumar K 
> Signed-off-by: Shaik Ameer Basha 
> ---
>  drivers/clk/samsung/clk-exynos5420.c   |6 --
>  include/dt-bindings/clock/exynos5420.h |2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Tomasz Figa 

--
Best regards,
Tomasz
--
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 1/3] ARM: EXYNOS: Add support for clock handling in power domain

[Tomasz Figa]

Hi,

On 26.05.2014 13:56, Shaik Ameer Basha wrote:
> From: Prathyush K 
> 
> While powering on/off a local powerdomain in exynos5 chipsets, the input
> clocks to each device gets modified. This behaviour is based on the
> SYSCLK_SYS_PWR_REG registers.
> E.g. SYSCLK_MFC_SYS_PWR_REG = 0x0, the parent of input clock to MFC
>  (aclk333) gets modified to oscclk
>   = 0x1, no change in clocks.
> The recommended value of SYSCLK_SYS_PWR_REG before power gating any
> domain is 0x0. So we must also restore the clocks while powering on a
> domain everytime.
> 
> This patch adds the framework for getting the required mux and parent clocks
> through a power domain device node. With this patch, while powering off
> a domain, parent is set to oscclk and while powering back on, its re-set
> to the correct parent which is as per the recommended pd on/off
> sequence.
> 
> Signed-off-by: Prathyush K 
> Signed-off-by: Andrew Bresticker 
> Signed-off-by: Arun Kumar K 
> Signed-off-by: Shaik Ameer Basha 
> ---
>  .../bindings/arm/exynos/power_domain.txt   |   20 +++
>  arch/arm/mach-exynos/pm_domains.c  |   59 
> +++-
>  2 files changed, 78 insertions(+), 1 deletion(-)
> 

Reviewed-by: Tomasz Figa 

--
Best regards,
Tomasz
--
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: dts: exynos4: Fix reg sizes of GIC

[Tomasz Figa]

Hi Kukjin,

On 30.05.2014 20:18, Kukjin Kim wrote:
> On 05/30/14 20:41, Tomasz Figa wrote:
>> Hi,
>>
>> On 23.05.2014 16:39, Tomasz Figa wrote:
>>> This patch fixes reg entry sizes in GIC node that were not large enough
>>> to cover whole regions.
>>>
>>> Signed-off-by: Tomasz Figa
>>> ---
>>>   arch/arm/boot/dts/exynos4.dtsi | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
>>> index b8ece4b..fbaf426 100644
>>> --- a/arch/arm/boot/dts/exynos4.dtsi
>>> +++ b/arch/arm/boot/dts/exynos4.dtsi
>>> @@ -113,7 +113,7 @@
>>> compatible = "arm,cortex-a9-gic";
>>> #interrupt-cells =<3>;
>>> interrupt-controller;
>>> -   reg =<0x1049 0x1000>,<0x1048 0x100>;
>>> +   reg =<0x1049 0x1>,<0x1048 0x1>;
>>> };
>>>
>>> combiner: interrupt-controller@1044 {
>>>
>>
>> Ping.
>>
> Yeah, SZ_64K is used for GIC_CPU and GIC_DIST before moving on DT 
> support. Applied, but I need to check this is required for stable tree...

Not sure if you already managed, but I think it might not be necessary
in stable trees, as the (correct) static mapping is still there and it
causes ioremap() to return correctly mapped area, ignoring the length
specified in device tree.

Best regards,
Tomasz
--
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: [RESEND PATCH v2 2/2] ASoC: samsung: drop support for legacy S3C24XX DMA API

[Tomasz Figa]

Hi,

On 01.06.2014 19:15, Vasily Khoruzhick wrote:
> Signed-off-by: Vasily Khoruzhick 
> ---
> v2: No changes
> 
>  sound/soc/samsung/Kconfig  |   3 -
>  sound/soc/samsung/Makefile |   2 -
>  sound/soc/samsung/dma.c| 460 
> -
>  3 files changed, 465 deletions(-)
>  delete mode 100644 sound/soc/samsung/dma.c
> 

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz
--
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: [RESEND PATCH v2 1/2] ASoC: samsung: s3c24{xx,12}-i2s: port to use generic dmaengine API

[Tomasz Figa]

Hi,

On 01.06.2014 19:15, Vasily Khoruzhick wrote:
> Use dmaengine instead of legacy s3c24xx DMA API for s3c24xx and s3c2412
> 
> Signed-off-by: Vasily Khoruzhick 
> ---
> v2: use hardcoded dma channel number
> 
>  sound/soc/samsung/Kconfig   |  9 +++
>  sound/soc/samsung/dmaengine.c   |  3 +++
>  sound/soc/samsung/s3c-i2s-v2.c  | 17 +
>  sound/soc/samsung/s3c2412-i2s.c | 47 +-
>  sound/soc/samsung/s3c24xx-i2s.c | 56 
> +++--
>  5 files changed, 56 insertions(+), 76 deletions(-)
> 

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz
--
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: mcpm: Don't rely on firmware's secondary_cpu_start

[Doug Anderson]

Chander,

On Tue, Jun 10, 2014 at 1:12 AM, Chander Kashyap
 wrote:
> On 10 June 2014 04:08, Lorenzo Pieralisi  wrote:
>> On Mon, Jun 09, 2014 at 06:03:31PM +0100, Doug Anderson wrote:
>>
>> [...]
>>
>>> Cold boot and resume from suspend are detected via various special
>>> flags in various special locations.  Resume from suspend looks at
>>> INFORM1 (0x10048004) for flags.  This register is 0 during a cold boot
>>> and has special values set by the kernel at resume time.
>>>
>>> It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help
>>> tell initial cold boot and secondary CPU bringup.
>>
>> Ok, thanks a lot. It looks like firmware paths should be ready to
>> detect cold vs warm boot, and hopefully do not rely on a specific
>> MPIDR to come up first out of power states.
>>
>>> > I am asking to check if on this platform CPUidle (where the notion of
>>> > primary CPU disappears) has a chance to run properly.
>>>
>>> I believe it should be possible, but we don't have CPUidle implemented
>>> in our current system.  Abhilash may be able to comment more.
>>
>
> Cpuidle is implemented for exynos5420, and is tested on chromebook.

My S-state knowledge is not strong, but I believe that Lorenzo's
questions matter if we're using S2 for CPUidle (where we actually turn
off power and hot unplug CPUs) but not when we're using S1 for CPUidle
(where we just enter WFI/WFE).

I believe that in ChromeOS we use S1 CPUidle and that it works fine.
We've never implemented S2 that I'm aware of.

-Doug
--
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 v3 6/6] ARM: EXYNOS: Refactoring to remove soc_is_exynos macros from exynos

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 09:20, Pankaj Dubey wrote:
> This patch enables chipid driver for ARCH_EXYNOS and refactors
> machine code as well as exynos cpufreq driver code for using
> chipid driver for identification of SoC ID and SoC rev.
> 
> This patch also updates DT binding information in exynos4 and
> exynos5 dtsi file. As to differentiate product id bit-mask we need
> separate compatible string for exynos4 and exynos5. Hoping this will
> be helpful in future as bit-mask and bit-shift bit may differ.
> Added binding information as well.

[snip]

> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index d0482c2..9d6ec84 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -20,13 +20,13 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
>  #include 
>  #include 
>  
> -#include 
>  #include 
>  
>  #include "common.h"
> @@ -59,7 +59,8 @@ static void __init exynos_smp_prepare_sram(void)
>  
>  static inline void __iomem *cpu_boot_reg_base(void)
>  {
> - if (soc_is_exynos4210() && samsung_rev() == EXYNOS4210_REV_1_1)
> + if (is_soc_id_compatible(EXYNOS4210) &&
> + is_soc_rev_compatible(EXYNOS4210_REV_1_1))

Well, how is this any better than what is already there? When adding a
new SoC, you will likely have to add another series of "||"s or "&&"s
with specific SoC IDs, when using this kind of API. This is not the point.

For now, all those things can be replaced simply with
of_machine_is_compatible() (except checks for samsung_rev()), but in the
end they need to be properly abstracted. The goal is to be able to add
support for new SoC, which requires identical steps to perform certain
things as already supported SoCs, without explicitly stating this in
driver code.

E.g. Exynos5250 is already supported, while Exynos5251 shows up. Let's
say it has exactly the same secondary CPU boot-up method as Exynos5250,
but to enter suspend mode, the same steps as on Exynos5260 have to be
performed. With any kind of per-SoC check API, you will have to add
soc_is_exynos5251() or is_soc_id_compatible(EXYNOS5251) in CPU bring-up
code and low-level suspend code, even though all the functional code is
already there.

Best regards,
Tomasz
--
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 v3 5/6] soc: samsung: exynos-chipid: Add Exynos Chipid driver support

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 09:20, Pankaj Dubey wrote:
> Exynos SoCs have Chipid, for identification of product IDs
> and SoC revistions. Till now we are using static macros
> such as soc_is_exynos and #ifdefs for run time identification
> of SoCs and their revisions. This is leading to add new Kconfig,
> soc_is_exynos definitions each time new SoC support is getting
> added. So this driver intends to provide initialization code
> all these functionalites and thus helping in removing macros.
> 
> Signed-off-by: Pankaj Dubey 
> ---
>  drivers/soc/Kconfig |1 +
>  drivers/soc/Makefile|1 +
>  drivers/soc/samsung/Kconfig |   10 +++
>  drivers/soc/samsung/Makefile|1 +
>  drivers/soc/samsung/exynos-chipid.c |  166 
> +++
>  include/linux/exynos-soc.h  |   46 ++
>  6 files changed, 225 insertions(+)
>  create mode 100644 drivers/soc/samsung/Kconfig
>  create mode 100644 drivers/soc/samsung/Makefile
>  create mode 100644 drivers/soc/samsung/exynos-chipid.c
>  create mode 100644 include/linux/exynos-soc.h
> 
> diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
> index 07a11be..86e52b1 100644
> --- a/drivers/soc/Kconfig
> +++ b/drivers/soc/Kconfig
> @@ -1,5 +1,6 @@
>  menu "SOC specific Drivers"
>  
>  source "drivers/soc/qcom/Kconfig"
> +source "drivers/soc/samsung/Kconfig"
>  
>  endmenu
> diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
> index 0f7c447..ee890aa 100644
> --- a/drivers/soc/Makefile
> +++ b/drivers/soc/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_ARCH_QCOM)  += qcom/
> +obj-$(CONFIG_ARCH_EXYNOS)+= samsung/

EXYNOS or samsung/?

Also this is just a single file, is it necessary to create a directory
for it?

> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> new file mode 100644
> index 000..dacc95d
> --- /dev/null
> +++ b/drivers/soc/samsung/Kconfig
> @@ -0,0 +1,10 @@
> +
> +# SAMSUNG Soc drivers
> +#
> +config EXYNOS_CHIPID
> + tristate "Support Exynos CHIPID"
> + default y
> + select SOC_BUS
> + depends on ARCH_EXYNOS || ARM64
> + help
> +   If you say Y here you get support for the Exynos CHIP id.

Do we need to have this user-selectable? Couldn't we just have it
selected by ARCH_EXYNOS and its 64-bit counterpart? AFAIK per-family
Kconfig symbols are already present (see ARCH_VEXPRESS and ARCH_XGENE in
arch/arm64/Kconfig), so it is not a problem.

> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
> new file mode 100644
> index 000..855ca05
> --- /dev/null
> +++ b/drivers/soc/samsung/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_EXYNOS_CHIPID)  += exynos-chipid.o
> diff --git a/drivers/soc/samsung/exynos-chipid.c 
> b/drivers/soc/samsung/exynos-chipid.c
> new file mode 100644
> index 000..b5bccd9
> --- /dev/null
> +++ b/drivers/soc/samsung/exynos-chipid.c
> @@ -0,0 +1,166 @@
> +/*
> + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> + * http://www.samsung.com/
> + *
> + * EXYNOS - CHIP ID support
> + * Author: Pankaj Dubey 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define EXYNOS4_SOC_MASK 0xFFFE0

Even though I can see the mask defined in current code to the value
above, the documentation states that the whole bit field [31:12] is used
for product ID, so it should be safe to use the same mask as for Exynos5.

> +#define EXYNOS5_SOC_MASK 0xF
> +
> +#define PROD_ID_SHIFT(12)
> +
> +static void __iomem  *exynos_chipid_base;
> +unsigned int exynos_soc_id = EXYNOS_SOC_UNKNOWN;

static?

> +enum exynos_soc_revision exynos_revision;

static?

> +
> +struct exynos_chipid_data {
> + unsigned int product_id_mask;
> + unsigned int product_id_shift;
> +};
> +
> +static struct exynos_chipid_data exynos4_chipid_data = {
> + .product_id_mask= EXYNOS4_SOC_MASK,
> + .product_id_shift   = PROD_ID_SHIFT,
> +};
> +
> +static struct exynos_chipid_data exynos5_chipid_data = {
> + .product_id_mask= EXYNOS5_SOC_MASK,
> + .product_id_shift   = PROD_ID_SHIFT,
> +};
> +
> +static struct of_device_id of_exynos_chipid_ids[] = {
> + {
> + .compatible = "samsung,exynos4-chipid",
> + .data   = (void *)&exynos4_chipid_data,
> + },
> + {
> + .compatible = "samsung,exynos5-chipid",
> + .data   = (void *)&exynos5_chipid_data,

We already have "samsung,exynos4210-chipid" compatible string defined
and used in existing device tree sources (and deployed DTBs).

> + },
> + {},
> +};
> +
> +bool is_soc_id_compatible(enum exynos_soc_id soc_id)
> +{
> + return soc_id == exynos_soc_id;
> +}
> +

Re: Problems booting exynos5420 with >1 CPU

[Catalin Marinas]

Hi Nico,

Sorry, I can't stay away from this thread ;)

On Tue, Jun 10, 2014 at 12:25:47AM -0400, Nicolas Pitre wrote:
> On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote:
> > 4) When I am talking about firmware I am talking about sequences that
> >are very close to HW (disabling C bit, cleaning caches, exiting
> >coherency). Erratas notwithstanding, they are being standardized at
> >ARM the best we can. They might even end up being implemented in HW
> >in the not so far future. I understand they are tricky, I understand
> >they take lots of time to implement them and to debug them, what I
> >want to say is that they are becoming standard and we _must_ reuse the
> >same code for all ARM platforms. You can implement them in MCPM (see
> >(1)) or in firmware (and please do not start painting me as firmware
> >hugger here, I am referring to standard power down sequences that
> >again, are very close to HW state machines 
> 
> That's where the disconnect lies.  On the one hand you say "I understand 
> they are tricky, I understand they take lots of time to implement them 
> and to debug them" and on the other hand you say "They might end up being 
> implemented in HW in the not so far future."  That simply makes no 
> economical sense at all!

It makes lots of sense, though not from a software maintainability
perspective. It would be nice if everything still looked like ARM7TDMI
but in the race for performance (vs power), hardware becomes more
complex and it's not just the CPU but adjacent parts like interconnects,
caches, asynchronous bridges, voltage shifters, memory controllers,
clocks/PLLs etc. Many of these are simply hidden from the high level OS
like Linux because the OS assumes certain configuration (e.g. access to
memory) and it's only the hardware itself that knows in what order they
can be turned on or off (when triggered explicitly by the OS or an
external event). Having an dedicated power controller (e.g. M-class
processor) to handle some of these is a rather flexible approach, other
bits require RTL (and usually impossible to update).

> When some operation is 1) tricky and takes time to debug, and 2) not 
> performance critical (no one is trying to get in and out of idle or 
> hibernation a billion times per second), then you should never ever put 
> such a thing in firmware, and hardware should be completely out of the 
> question!

I agree that things can go wrong (both in hardware and software, no
matter where it runs) but please don't think that such power
architecture has been specifically engineered to hide the hardware from
Linux. It's a necessity for complex systems and the optimal solution is
not always simplification (it's not just ARM+vendors doing this, just
look at the power model of modern x86 processors, hidden nicely from the
software behind a few registers while making things harder for scheduler
which cannot rely on a constant performance level; but it's a trade-off
they are happy to make).

> >and more importantly if they
> >HAVE to run in secure world that's the only solution we have unless you
> >want to split race conditions between kernel and secure world).
> 
> If they HAVE to run in secure world then your secure world architecture 
> is simply misdesigned, period.  Someone must have ignored the economics 
> of modern software development to have come up with this.

That's the trade-off between software complexity and hardware cost,
gates, power consumption. You can do proper physical separation of the
secure services but this would require a separate CPU that is rarely
used and adds to the overall SoC cost. On large scale hardware
deployment, it's exactly economics that matter and these translate into
hardware cost. The software cost is irrelevant here, whether we like it
or not.

-- 
Catalin
--
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 1/2] ARM: exynos: cleanup kconfig option display

[Rob Herring]

From: Rob Herring 

The addition of Exynos to multi-platform configs creates a mess of config
options with options appearing before the Exynos config option. This is
due to arch/arm/plat-samsung/Kconfig being included out of order with the
other Samsung platform kconfig files. Reorder the kconfig files and move
all the options into a sub-menu. Some of the options are dead, so remove
those as well.

Signed-off-by: Rob Herring 
Cc: Ben Dooks 
Cc: Kukjin Kim 
Cc: linux-samsung-soc@vger.kernel.org
---
 arch/arm/Kconfig  |  3 +--
 arch/arm/plat-samsung/Kconfig | 17 +++--
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 87b63fd..cbf0c37 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1009,8 +1009,6 @@ source "arch/arm/mach-rockchip/Kconfig"
 
 source "arch/arm/mach-sa1100/Kconfig"
 
-source "arch/arm/plat-samsung/Kconfig"
-
 source "arch/arm/mach-socfpga/Kconfig"
 
 source "arch/arm/mach-spear/Kconfig"
@@ -1028,6 +1026,7 @@ source "arch/arm/mach-s5pc100/Kconfig"
 source "arch/arm/mach-s5pv210/Kconfig"
 
 source "arch/arm/mach-exynos/Kconfig"
+source "arch/arm/plat-samsung/Kconfig"
 
 source "arch/arm/mach-shmobile/Kconfig"
 
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 243dfcb..ac032cd 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -35,27 +35,15 @@ config SAMSUNG_PM
  Base platform power management code for samsung code
 
 if PLAT_SAMSUNG
+menu "Samsung Common options"
 
 # boot configurations
 
 comment "Boot options"
 
-config S3C_BOOT_ERROR_RESET
-   bool "S3C Reboot on decompression error"
-   help
- Say y here to use the watchdog to reset the system if the
- kernel decompressor detects an error during decompression.
-
-config S3C_BOOT_UART_FORCE_FIFO
-   bool "Force UART FIFO on during boot process"
-   default y
-   help
- Say Y here to force the UART FIFOs on during the kernel
-uncompressor
-
-
 config S3C_LOWLEVEL_UART_PORT
int "S3C UART to use for low-level messages"
+   depends on ARCH_S3C64XX
default 0
help
  Choice of which UART port to use for the low-level messages,
@@ -503,4 +491,5 @@ config DEBUG_S3C_UART
default "2" if DEBUG_S3C_UART2
default "3" if DEBUG_S3C_UART3
 
+endmenu
 endif
-- 
1.9.1

--
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 v3 3/6] ARM: EXYNOS: Remove soc_is_exynos4/5 from exynos.c

[Arnd Bergmann]

On Tuesday 10 June 2014 15:55:09 Tomasz Figa wrote:
> On 10.05.2014 09:20, Pankaj Dubey wrote:
> > This patch removes usage of soc_is_exynos4/5 from exynos.c.
> > For this we need to separate machine descriptors for exynos4
> > and exynos5. While doing this patch does some consolidation also.
> > 
> > CC: Russell King 
> > CC: Thomas Abraham 
> > Signed-off-by: Pankaj Dubey 
> > ---
> >  arch/arm/mach-exynos/exynos.c|   52 
> > +++---
> >  arch/arm/plat-samsung/include/plat/cpu.h |3 --
> >  2 files changed, 33 insertions(+), 22 deletions(-)
> > 
> 
> I don't think this is needed. All those static mappings should just go
> away, which will also eliminate the need to have separate machine
> descriptor or soc_is_exynos{4,5}() macros.

I think I suggested doing this patch as an intermediate step to
get rid of the macros first, so we don't get any new users.
I agree that we should also remove the static mappings eventually,
and once we do that, the machine descriptors can be unified again.

I have not checked which of the map_desc entries are currently used
at all, or what it would take to remove them.

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 v3 4/6] ARM: EXYNOS: Remove unused header inclusion from hotplug.c

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 09:20, Pankaj Dubey wrote:
> This patch removed "plat/cpu.h" inclusion from hotplug.c as it
> is not required.
> 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/hotplug.c |2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> index 0243ef3..5e19601 100644
> --- a/arch/arm/mach-exynos/hotplug.c
> +++ b/arch/arm/mach-exynos/hotplug.c
> @@ -19,8 +19,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "common.h"
>  #include "regs-pmu.h"
>  
> 

I don't see this header being included in current linux-next.

Best regards,
Tomasz
--
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 v3 3/6] ARM: EXYNOS: Remove soc_is_exynos4/5 from exynos.c

[Tomasz Figa]

On 10.05.2014 09:20, Pankaj Dubey wrote:
> This patch removes usage of soc_is_exynos4/5 from exynos.c.
> For this we need to separate machine descriptors for exynos4
> and exynos5. While doing this patch does some consolidation also.
> 
> CC: Russell King 
> CC: Thomas Abraham 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/exynos.c|   52 
> +++---
>  arch/arm/plat-samsung/include/plat/cpu.h |3 --
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 

I don't think this is needed. All those static mappings should just go
away, which will also eliminate the need to have separate machine
descriptor or soc_is_exynos{4,5}() macros.

Best regards,
Tomasz
--
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 v3 2/6] ARM: EXYNOS: Remove i2c sys configuration related code

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 09:20, Pankaj Dubey wrote:
> Since all these code has been moved into i2c driver, now we can
> safely remove them from machine files.
> 
> CC: Russell King 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/exynos.c   |   38 
> +--
>  arch/arm/mach-exynos/include/mach/map.h |3 ---
>  arch/arm/mach-exynos/pm.c   |   10 
>  arch/arm/mach-exynos/regs-sys.h |   22 --
>  4 files changed, 1 insertion(+), 72 deletions(-)
>  delete mode 100644 arch/arm/mach-exynos/regs-sys.h
> 
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 59eb1f1..09063ee 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -29,11 +29,11 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  #include "common.h"
>  #include "mfc.h"
>  #include "regs-pmu.h"
> -#include "regs-sys.h"
>  
>  #define L2_AUX_VAL 0x7C470001
>  #define L2_AUX_MASK 0xC200
> @@ -42,11 +42,6 @@ static struct regmap *exynos_pmu_regmap;
>  
>  static struct map_desc exynos4_iodesc[] __initdata = {
>   {
> - .virtual= (unsigned long)S3C_VA_SYS,
> - .pfn= __phys_to_pfn(EXYNOS4_PA_SYSCON),
> - .length = SZ_64K,
> - .type   = MT_DEVICE,

Rest of this patch doesn't touch anything related to Exynos4, but I
boot-tested removing this on Exynos4412-based Trats2 board and it works, so

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz
--
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 v3 1/6] i2c: s3c2410: Handle i2c sys_cfg register in i2c driver

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 09:20, Pankaj Dubey wrote:
> Let's handle i2c interrupt re-configuration in i2c driver. This will
> help us in removing some soc specific checks from machine files.
> Since only Exynos5250, and Exynos5420 need to do this, added syscon
> based phandle to i2c device nodes of respective SoC DT files.
> Also handle saving and restoring of SYS_I2C_CFG register during
> suspend and resume of i2c driver. This will help in removing soc
> specific check from mach-exynos/pm.c.
> 
> CC: Rob Herring 
> CC: Randy Dunlap 
> CC: Wolfram Sang 
> CC: Russell King 
> CC: devicet...@vger.kernel.org
> CC: linux-...@vger.kernel.org
> CC: linux-...@vger.kernel.org
> Signed-off-by: Pankaj Dubey 
> ---
>  .../devicetree/bindings/arm/samsung/sysreg.txt |1 +
>  arch/arm/boot/dts/exynos5.dtsi |5 +++
>  arch/arm/boot/dts/exynos5250.dtsi  |4 +++
>  arch/arm/boot/dts/exynos5420.dtsi  |4 +++

What about Exynos5410 and Exynos5800?

>  drivers/i2c/busses/i2c-s3c2410.c   |   32 
> 
>  5 files changed, 46 insertions(+)

[snip]

> diff --git a/drivers/i2c/busses/i2c-s3c2410.c 
> b/drivers/i2c/busses/i2c-s3c2410.c
> index ae44910..e707062 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -39,6 +39,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include 
>  
> @@ -91,6 +93,9 @@
>  /* Max time to wait for bus to become idle after a xfer (in us) */
>  #define S3C2410_IDLE_TIMEOUT 5000
>  
> +/* Exynos5 Sysreg offset */
> +#define EXYNOS5_SYS_I2C_CFG  0x0234
> +
>  /* i2c controller state */
>  enum s3c24xx_i2c_state {
>   STATE_IDLE,
> @@ -127,6 +132,8 @@ struct s3c24xx_i2c {
>  #if defined(CONFIG_ARM_S3C24XX_CPUFREQ)
>   struct notifier_block   freq_transition;
>  #endif
> + struct regmap   *sysreg;
> + unsigned intsys_i2s_cfg;

typo: s/i2s/i2c/

>  };
>  
>  static struct platform_device_id s3c24xx_driver_ids[] = {
> @@ -1075,6 +1082,8 @@ static void
>  s3c24xx_i2c_parse_dt(struct device_node *np, struct s3c24xx_i2c *i2c)
>  {
>   struct s3c2410_platform_i2c *pdata = i2c->pdata;
> + u32 val = 0;
> + int id;
>  
>   if (!np)
>   return;
> @@ -1084,6 +1093,23 @@ s3c24xx_i2c_parse_dt(struct device_node *np, struct 
> s3c24xx_i2c *i2c)
>   of_property_read_u32(np, "samsung,i2c-slave-addr", &pdata->slave_addr);
>   of_property_read_u32(np, "samsung,i2c-max-bus-freq",
>   (u32 *)&pdata->frequency);
> + /*
> +  * Exynos5's legacy i2c controller and new high speed i2c
> +  * controller have muxed interrupt sources. By default the

What do you mean by "by default"? Is it a setting from the bootloader or
reset value?

Probably to ensure that the mux is set correctly, same thing should be
also added to hsi2c driver.

> +  * interrupts for 4-channel HS-I2C controller are enabled.
> +  * If node for first four channels of legacy i2c controller
> +  * are available then re-configure the interrupts via the
> +  * system register.
> +  */
> + id = of_alias_get_id(np, "i2c");
> + i2c->sysreg = syscon_regmap_lookup_by_phandle(np,
> + "samsung,syscon-phandle");
> + if (IS_ERR(i2c->sysreg)) {
> + /* As this is not compulsory do not return error */
> + pr_info("i2c-%d skipping re-configuration of interrutps\n", id);
> + return;
> + }
> + regmap_update_bits(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, BIT(id), val);

As Wolfram pointed, val can be replaced with immediate 0 here.

>  }
>  #else
>  static void
> @@ -1268,6 +1294,9 @@ static int s3c24xx_i2c_suspend_noirq(struct device *dev)
>  
>   i2c->suspended = 1;
>  
> + if (!IS_ERR(i2c->sysreg))
> + regmap_read(i2c->sysreg, EXYNOS5_SYS_I2C_CFG, 
> &i2c->sys_i2s_cfg);
> +
>   return 0;
>  }
>  
> @@ -1276,6 +1305,9 @@ static int s3c24xx_i2c_resume(struct device *dev)
>   struct platform_device *pdev = to_platform_device(dev);
>   struct s3c24xx_i2c *i2c = platform_get_drvdata(pdev);
>  
> + if (!IS_ERR(i2c->sysreg))
> + regmap_write(i2c->sysreg, i2c->sys_i2s_cfg, 
> EXYNOS5_SYS_I2C_CFG);

According to include/linux/regmap.h:

int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);

So in your code reg is swapped with val.

Best regards,
Tomasz
--
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: mark machine descriptor functions static

[Tomasz Figa]

Hi Olof,

On 03.06.2014 06:57, Olof Johansson wrote:
> There's no reason to export these functions, and I have no idea why
> they have over time ended up in the header file. As a result, none of
> the checker tools caught it (i.e. sparse was silent on it).
> 
> Signed-off-by: Olof Johansson 
> ---
> 
> I'm guessing with all the churn right now this might not apply, so it
> might need manual application around -rc1.
> 
> There's definitely room for more cleanup in common.h.
> 
>  arch/arm/mach-exynos/common.h |6 --
>  arch/arm/mach-exynos/exynos.c |   10 +-
>  2 files changed, 5 insertions(+), 11 deletions(-)
> 

Similar patch and few other clean-up patches were posted some time ago
on the list as a part of PMU rework series, but somehow got missed. They
can be applied independently from rest of the series which is still
under discussion.

Best regards,
Tomasz
--
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: move sysram info to exynos.c

[Tomasz Figa]

Hi Olof,

On 03.06.2014 06:47, Olof Johansson wrote:
> This solves a problem with building with CONFIG_SMP=n due to missing
> sysram_base_addr (or sysram_ns_base_addr) variables.
> 
> The new setup method is more awkward than I'd like for it to be, but
> it can't be done in init_early() since ioremap is not yet available,
> but it needs to happen before SMP.
> 
> Reported-by: Russell King 
> Cc: Kukjin Kim 
> Cc: Tomasz Figa 
> Cc: Daniel Lezcano 
> Signed-off-by: Olof Johansson 
> ---
> 
> I'm not entirely happy with the solution here, especially the dual
> call path. The platsmp.c->exynos.c call isn't ideal either but I'm less
> worried about that. Seemed overkill to create a new c file just for this.
> 
> I've tested to make sure this still works on arndale, and the build
> errors as reported by Russell are definitely gone.

In general, I'm okay with this patch, so if nobody is willing to find a
better way feel free to add my Reviewed-by.

> 
> Better ideas welcome.
> 

Probably the best idea would be to find a way to always call
exynos_sysram_init() after DT unflattening and before SMP
initialization. Some platforms (e.g. tegra) abuse .init_irq callback for
this, e.g.:

mach-tegra/tegra.c:
static void __init tegra_dt_init_irq(void)
{
tegra_pmc_init_irq();
tegra_init_irq();
irqchip_init();
tegra_legacy_irq_syscore_init();
}

We could do the same on Exynos if this is considered better.

Best regards,
Tomasz
--
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: drm/exynos: consider deferred probe case

[Dan Carpenter]

Hello Inki Dae,

The patch df5225bc9a87: "drm/exynos: consider deferred probe case"
from May 29, 2014, leads to the following static checker warning:

drivers/gpu/drm/exynos/exynos_drm_fimd.c:996 fimd_probe()
warn: 'ctx->display' isn't an ERR_PTR

drivers/gpu/drm/exynos/exynos_drm_fimd.c
   994  
   995  ctx->display = exynos_dpi_probe(dev);
   996  if (IS_ERR(ctx->display))
   997  return PTR_ERR(ctx->display);
   998  

Smatch is complaining because my config has CONFIG_DRM_EXYNOS_DPI
disabled.

1) If CONFIG_DRM_EXYNOS_DPI isn't enabled, we still return "0".  That
will cause a Sparse warning.

2) Also there are still a number of checks for "if (ctx->display)".
Those things are weird to me, are those checks to see
CONFIG_DRM_EXYNOS_DPI is enabled or are they checking that
exynos_dpi_probe() succeeded?

regards,
dan carpenter
--
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 v6 0/6] add cpuidle support for Exynos5420

[Chander M. Kashyap]

On Thu, May 29, 2014 at 10:07 AM, Chander Kashyap
 wrote:
> On 28 May 2014 14:32, Daniel Lezcano  wrote:
>> On 05/28/2014 06:35 AM, Kukjin Kim wrote:
>>>
>>> Chander Kashyap wrote:


 On 26 May 2014 15:59, Tomasz Figa  wrote:
>
> Hi Chander,
>
> On 16.05.2014 10:03, Chander Kashyap wrote:
>>
>> Exynos5420 is a big-little Soc from Samsung. It has 4 A15 and 4 A7

 cores.
>>
>>
>> This patchset adds cpuidle support for Exynos5420 SoC based on
>> generic big.little cpuidle driver.
>>
>> Tested on SMDK5420.
>>
>> This patch set depends on:
>>1. [PATCH 0/5] MCPM backend for Exynos5420
>>   http://www.spinics.net/lists/arm-kernel/msg331100.html
>> Changelog is in respective patches.
>> Chander Kashyap (5):
>>driver: cpuidle-big-little: add of_device_id structure
>>arm: exynos: add generic function to calculate cpu number
>>cpuidle: config: Add ARCH_EXYNOS entry to select cpuidle-big-little
>>  driver
>>driver: cpuidle: cpuidle-big-little: init driver for Exynos5420
>>exynos: cpuidle: do not allow cpuidle registration for Exynos5420
>>mcpm: exynos: populate suspend and powered_up callbacks
>>
>>   arch/arm/mach-exynos/exynos.c|4 +++-
>>   arch/arm/mach-exynos/mcpm-exynos.c   |   36

 ++
>>
>>   arch/arm/mach-exynos/regs-pmu.h  |9 +
>>   drivers/cpuidle/Kconfig.arm  |2 +-
>>   drivers/cpuidle/cpuidle-big_little.c |   12 +++-
>>   5 files changed, 60 insertions(+), 3 deletions(-)
>>
>
> For the whole series,
>
> Reviewed-by: Tomasz Figa 


 Thanks Tomasz.

 Dear Kukjin,
 Can you take these patches.
>
>
>>> When I looked at this series quickly, looks good to me but I need to get
>>> ack from cpuidle maintainer Rafael or Daniel.
>>
>>
>> Acked the different cpuidle bits.
>
> Hi Kukjin,
> Can you take it now?

ping
Sorry for previous html messages.

>
>
>>
>> Thanks
>>   -- Daniel
>>
>>
>> --
>>   Linaro.org │ Open source software for ARM SoCs
>>
>> Follow Linaro:   Facebook |
>>  Twitter |
>>  Blog
>>
>
>
>
> --
> with warm regards,
> Chander Kashyap



-- 
thanks and regards,
Chander M. Kashyap
Contact Number: +918123738320
--- TENSION LENE KA NAHI, DENE KE-
--
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 v4 02/11] ARM: EXYNOS: Move cpufreq and cpuidle device registration to init_machine

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 08:56, Pankaj Dubey wrote:
> As exynos_cpuidle_init and exynos_cpufreq_init function have just one lines
> of code for registering platform devices. We can move these lines to
> exynos_dt_machine_init and delete exynos_cpuidle_init and exynos_cpufreq_init
> function. This will help in reducing lines of code in exynos.c, making it
> more cleaner.
> 
> Suggested-by: Tomasz Figa 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/exynos.c |   19 ---
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 

Reviewed-by: Tomasz Figa 

Btw. This and other simple clean-up patches from this series could be
applied separately, without re-spinning them every time with this series.

Best regards,
Tomasz
--
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 v4 01/11] ARM: EXYNOS: Make exynos machine_ops as static

[Tomasz Figa]

Hi Pankaj,

On 10.05.2014 08:56, Pankaj Dubey wrote:
> As machine function ops are used only in this file let's make
> them static. Also remove unused and unwanted declarations from
> common.h.
> 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/common.h |8 
>  arch/arm/mach-exynos/exynos.c |6 +++---
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz
--
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 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Ch]

Hello Sylwester,

Thanks for the review.
On 10 June 2014 16:09, Sylwester Nawrocki  wrote:
> On 10/06/14 12:08, Naveen Krishna Chatradhi wrote:
>> Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
>> defined under "controller-data" node under each slave node.
>>
>> &spi_x {
>>   cs-gpios <>;
>>   ...
>>   slave_node {
>>
>>   controller-data {
>>   cs-gpio = <>;
>>   ...
>>   };
>>   ...
>>   };
>>   ...
>> };
>>
>> Where as, SPI core and many other drivers uses "cs-gpios" for
>> from device tree node.
>>
>> Hence, make changes in spi-s3c64xx.c driver to make use of
>> "cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
>> slaves "controller-data"(child) node.
>>
>> Also updates the Device tree Documentation.
>>
>> Signed-off-by: Naveen Krishna Chatradhi 
>> Cc: Javier Martinez Canillas 
>> Cc: Doug Anderson 
>> ---
>> Changes since v1:
>> 1. fixed a compilation warning thus squashing the other patch into this.
>> 2. Updated device tree description in spi-samsung.txt
>>
>>  .../devicetree/bindings/spi/spi-samsung.txt|8 ++-
>>  drivers/spi/spi-s3c64xx.c  |   56 
>> 
>>  2 files changed, 38 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt 
>> b/Documentation/devicetree/bindings/spi/spi-samsung.txt
>> index 86aa061..13bfcb5 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
>> +++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
>> @@ -42,15 +42,13 @@ Optional Board Specific Properties:
>>  - num-cs: Specifies the number of chip select lines supported. If
>>not specified, the default number of chip select lines is set to 1.
>>
>> +- cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
>> +
>>  SPI Controller specific data in SPI slave nodes:
>>
>>  - The spi slave nodes should provide the following information which is 
>> required
>>by the spi controller.
>>
>> -  - cs-gpio: A gpio specifier that specifies the gpio line used as
>> -the slave select line by the spi controller. The format of the gpio
>> -specifier depends on the gpio controller.
>> -
>>- samsung,spi-feedback-delay: The sampling phase shift to be applied on 
>> the
>>  miso line (to account for any lag in the miso line). The following are 
>> the
>>  valid values.
>> @@ -85,6 +83,7 @@ Example:
>>   #size-cells = <0>;
>>   pinctrl-names = "default";
>>   pinctrl-0 = <&spi0_bus>;
>> + cs-gpios = <&gpa2 5 1 0 3>;
>
> While at it, please change the GPIO specifier format, this one is not
> valid any more.
Sure,  i will club it with the other comments.
>
>>   w25q80bw@0 {
>>   #address-cells = <1>;
>> @@ -94,7 +93,6 @@ Example:
>>   spi-max-frequency = <1>;
>>
>>   controller-data {
>> - cs-gpio = <&gpa2 5 1 0 3>;
>>   samsung,spi-feedback-delay = <0>;
>>   };
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 75a5696..4594dde 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -750,47 +750,56 @@ static int s3c64xx_spi_transfer_one(struct spi_master 
>> *master,
>>  }
>>
>>  static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
>> - struct spi_device *spi)
>> + struct spi_device *spi,
>> + struct s3c64xx_spi_csinfo *cs)
>>  {
>> - struct s3c64xx_spi_csinfo *cs;
>> - struct device_node *slave_np, *data_np = NULL;
>> - struct s3c64xx_spi_driver_data *sdd;
>> + struct device_node *data_np = NULL;
>>   u32 fb_delay = 0;
>>
>> - sdd = spi_master_get_devdata(spi->master);
>> - slave_np = spi->dev.of_node;
>> - if (!slave_np) {
>> - dev_err(&spi->dev, "device node not found\n");
>> + data_np = of_get_child_by_name(spi->dev.of_node, "controller-data");
>> + if (!data_np) {
>> + dev_err(&spi->dev, "child node 'controller-data' not found\n");
>>   return ERR_PTR(-EINVAL);
>>   }
>>
>> - data_np = of_get_child_by_name(slave_np, "controller-data");
>> - if (!data_np) {
>> - dev_err(&spi->dev, "child node 'controller-data' not found\n");
>> + of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
>> + cs->fb_delay = fb_delay;
>> + of_node_put(data_np);
>> +
>> + return cs;
>> +}
>> +
>> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
>> *spi)
>> +{
>> + struct device_node *parent_np = NULL;
>> + struct s3c64xx_spi_driver_data *sdd;
>> + struct s3c64xx_spi_csinfo *cs;
>> +
>> + parent_np = of_get_parent(spi->dev.of_node);
>> + if (!parent_np

Re: [PATCH 2/5] regulator: Add driver for Maxim 77802 PMIC regulators

[Mark Brown]

On Tue, Jun 10, 2014 at 01:29:17AM +0200, Javier Martinez Canillas wrote:
> On 06/09/2014 09:38 PM, Mark Brown wrote:
> > On Mon, Jun 09, 2014 at 11:37:47AM +0200, Javier Martinez Canillas wrote:

> >> +  case REGULATOR_MODE_STANDBY:/* switch off */
> >> +  if (id != MAX77802_LDO1 && id != MAX77802_LDO20 &&
> >> +  id != MAX77802_LDO21 && id != MAX77802_LDO3) {
> >> +  val = MAX77802_OPMODE_STANDBY;
> >> +  break;
> >> +  }
> >> +  /* no break */

> > This sounds very broken...

> The problem is that not all regulators supports the same operational modes. 
> For
> instance regulators LDO 1, 20, 21 and 3 does not support 
> REGULATOR_MODE_STANDBY
> so if the condition is not met a break is not needed since the default case is
> to warn that the mode is not supported.

No, it's the "switch off" comment...

> But I'll rework that logic on v2 to make it cleaner and have a break on each
> case and don't rely on case cascading.

...though you should also consider splitting things up so you have
separate ops for separate regulators if they behave differently.


signature.asc
Description: Digital signature

Re: [PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Sylwester Nawrocki]

On 10/06/14 12:08, Naveen Krishna Chatradhi wrote:
> Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
> defined under "controller-data" node under each slave node.
> 
> &spi_x {
>   cs-gpios <>;
>   ...
>   slave_node {
> 
>   controller-data {
>   cs-gpio = <>;
>   ...
>   };
>   ...
>   };
>   ...
> };
> 
> Where as, SPI core and many other drivers uses "cs-gpios" for
> from device tree node.
> 
> Hence, make changes in spi-s3c64xx.c driver to make use of
> "cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
> slaves "controller-data"(child) node.
> 
> Also updates the Device tree Documentation.
> 
> Signed-off-by: Naveen Krishna Chatradhi 
> Cc: Javier Martinez Canillas 
> Cc: Doug Anderson 
> ---
> Changes since v1:
> 1. fixed a compilation warning thus squashing the other patch into this.
> 2. Updated device tree description in spi-samsung.txt
> 
>  .../devicetree/bindings/spi/spi-samsung.txt|8 ++-
>  drivers/spi/spi-s3c64xx.c  |   56 
> 
>  2 files changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt 
> b/Documentation/devicetree/bindings/spi/spi-samsung.txt
> index 86aa061..13bfcb5 100644
> --- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
> @@ -42,15 +42,13 @@ Optional Board Specific Properties:
>  - num-cs: Specifies the number of chip select lines supported. If
>not specified, the default number of chip select lines is set to 1.
>  
> +- cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
> +
>  SPI Controller specific data in SPI slave nodes:
>  
>  - The spi slave nodes should provide the following information which is 
> required
>by the spi controller.
>  
> -  - cs-gpio: A gpio specifier that specifies the gpio line used as
> -the slave select line by the spi controller. The format of the gpio
> -specifier depends on the gpio controller.
> -
>- samsung,spi-feedback-delay: The sampling phase shift to be applied on the
>  miso line (to account for any lag in the miso line). The following are 
> the
>  valid values.
> @@ -85,6 +83,7 @@ Example:
>   #size-cells = <0>;
>   pinctrl-names = "default";
>   pinctrl-0 = <&spi0_bus>;
> + cs-gpios = <&gpa2 5 1 0 3>;

While at it, please change the GPIO specifier format, this one is not
valid any more.

>   w25q80bw@0 {
>   #address-cells = <1>;
> @@ -94,7 +93,6 @@ Example:
>   spi-max-frequency = <1>;
>  
>   controller-data {
> - cs-gpio = <&gpa2 5 1 0 3>;
>   samsung,spi-feedback-delay = <0>;
>   };
>  
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 75a5696..4594dde 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -750,47 +750,56 @@ static int s3c64xx_spi_transfer_one(struct spi_master 
> *master,
>  }
>  
>  static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
> - struct spi_device *spi)
> + struct spi_device *spi,
> + struct s3c64xx_spi_csinfo *cs)
>  {
> - struct s3c64xx_spi_csinfo *cs;
> - struct device_node *slave_np, *data_np = NULL;
> - struct s3c64xx_spi_driver_data *sdd;
> + struct device_node *data_np = NULL;
>   u32 fb_delay = 0;
>  
> - sdd = spi_master_get_devdata(spi->master);
> - slave_np = spi->dev.of_node;
> - if (!slave_np) {
> - dev_err(&spi->dev, "device node not found\n");
> + data_np = of_get_child_by_name(spi->dev.of_node, "controller-data");
> + if (!data_np) {
> + dev_err(&spi->dev, "child node 'controller-data' not found\n");
>   return ERR_PTR(-EINVAL);
>   }
>  
> - data_np = of_get_child_by_name(slave_np, "controller-data");
> - if (!data_np) {
> - dev_err(&spi->dev, "child node 'controller-data' not found\n");
> + of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
> + cs->fb_delay = fb_delay;
> + of_node_put(data_np);
> +
> + return cs;
> +}
> +
> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
> *spi)
> +{
> + struct device_node *parent_np = NULL;
> + struct s3c64xx_spi_driver_data *sdd;
> + struct s3c64xx_spi_csinfo *cs;
> +
> + parent_np = of_get_parent(spi->dev.of_node);
> + if (!parent_np) {
> + dev_err(&spi->dev, "Parent node not found\n");
>   return ERR_PTR(-EINVAL);
>   }
>  
> + sdd = spi_master_get_devdata(spi->master);
> +
>   cs = kzalloc(sizeof(*cs), GFP_KERNEL);
>   if (!cs) {
> -   

[PATCH 1/2 v2] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Chatradhi]

Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
defined under "controller-data" node under each slave node.

&spi_x {
cs-gpios <>;
...
slave_node {

controller-data {
cs-gpio = <>;
...
};
...
};
...
};

Where as, SPI core and many other drivers uses "cs-gpios" for
from device tree node.

Hence, make changes in spi-s3c64xx.c driver to make use of
"cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
slaves "controller-data"(child) node.

Also updates the Device tree Documentation.

Signed-off-by: Naveen Krishna Chatradhi 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
---
Changes since v1:
1. fixed a compilation warning thus squashing the other patch into this.
2. Updated device tree description in spi-samsung.txt

 .../devicetree/bindings/spi/spi-samsung.txt|8 ++-
 drivers/spi/spi-s3c64xx.c  |   56 
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt 
b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 86aa061..13bfcb5 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -42,15 +42,13 @@ Optional Board Specific Properties:
 - num-cs: Specifies the number of chip select lines supported. If
   not specified, the default number of chip select lines is set to 1.
 
+- cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is 
required
   by the spi controller.
 
-  - cs-gpio: A gpio specifier that specifies the gpio line used as
-the slave select line by the spi controller. The format of the gpio
-specifier depends on the gpio controller.
-
   - samsung,spi-feedback-delay: The sampling phase shift to be applied on the
 miso line (to account for any lag in the miso line). The following are the
 valid values.
@@ -85,6 +83,7 @@ Example:
#size-cells = <0>;
pinctrl-names = "default";
pinctrl-0 = <&spi0_bus>;
+   cs-gpios = <&gpa2 5 1 0 3>;
 
w25q80bw@0 {
#address-cells = <1>;
@@ -94,7 +93,6 @@ Example:
spi-max-frequency = <1>;
 
controller-data {
-   cs-gpio = <&gpa2 5 1 0 3>;
samsung,spi-feedback-delay = <0>;
};
 
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 75a5696..4594dde 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -750,47 +750,56 @@ static int s3c64xx_spi_transfer_one(struct spi_master 
*master,
 }
 
 static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
-   struct spi_device *spi)
+   struct spi_device *spi,
+   struct s3c64xx_spi_csinfo *cs)
 {
-   struct s3c64xx_spi_csinfo *cs;
-   struct device_node *slave_np, *data_np = NULL;
-   struct s3c64xx_spi_driver_data *sdd;
+   struct device_node *data_np = NULL;
u32 fb_delay = 0;
 
-   sdd = spi_master_get_devdata(spi->master);
-   slave_np = spi->dev.of_node;
-   if (!slave_np) {
-   dev_err(&spi->dev, "device node not found\n");
+   data_np = of_get_child_by_name(spi->dev.of_node, "controller-data");
+   if (!data_np) {
+   dev_err(&spi->dev, "child node 'controller-data' not found\n");
return ERR_PTR(-EINVAL);
}
 
-   data_np = of_get_child_by_name(slave_np, "controller-data");
-   if (!data_np) {
-   dev_err(&spi->dev, "child node 'controller-data' not found\n");
+   of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
+   cs->fb_delay = fb_delay;
+   of_node_put(data_np);
+
+   return cs;
+}
+
+static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device *spi)
+{
+   struct device_node *parent_np = NULL;
+   struct s3c64xx_spi_driver_data *sdd;
+   struct s3c64xx_spi_csinfo *cs;
+
+   parent_np = of_get_parent(spi->dev.of_node);
+   if (!parent_np) {
+   dev_err(&spi->dev, "Parent node not found\n");
return ERR_PTR(-EINVAL);
}
 
+   sdd = spi_master_get_devdata(spi->master);
+
cs = kzalloc(sizeof(*cs), GFP_KERNEL);
if (!cs) {
-   of_node_put(data_np);
+   of_node_put(parent_np);
return ERR_PTR(-ENOMEM);
}
 
/* The CS line is asserted/deasserted by the gpio pin */
if (sdd->cs_gpio)
-   cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
+   c

[PATCH 0/2 v2] spi: s3c64xx: use "cs-gpios" in spi node instead of "cs-gpio"

[Naveen Krishna Chatradhi]

Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
defined under "controller-data" node under each slave node.

&spi_x {
cs-gpios <>;
...
slave_node {

controller-data {
cs-gpio = <>;
...
};
...
};
...
};

Where as, SPI core and many other drivers uses "cs-gpios" for
from device tree node.

Hence, make changes in spi-s3c64xx.c driver to make use of
"cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
slaves "controller-data"(child) node.

Also, fixes a compilation warning and corrects the DTS nodes for
Exynos4210 based SMDKv310, Exynos4412 based Trats2, Exynos5250 based
SMDK5250 boards.

Naveen Krishna Chatradhi (2):
based on for-next branch of spi.git
  spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

based on for-next branch of linuxsamsung.git
  ARM: DTS: move "cs-gpio" from "controller-data" to under spi node

 .../devicetree/bindings/spi/spi-samsung.txt|8 ++-
 arch/arm/boot/dts/exynos4210-smdkv310.dts  |2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts|2 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts  |2 +-
 drivers/spi/spi-s3c64xx.c  |   56 
 5 files changed, 41 insertions(+), 29 deletions(-)

-- 
1.7.9.5

--
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 2/2 v2] ARM: DTS: move "cs-gpio" from "controller-data" to under spi node

[Naveen Krishna Chatradhi]

This patch moves the "cs-gpio" field from "controller-data" child
node to under the spi device node.

Respective changes are preposed to spi-s3c64xx.c driver.

Signed-off-by: Naveen Krishna Chatradhi 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
---
Changes since v1:
None

 arch/arm/boot/dts/exynos4210-smdkv310.dts |2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts   |2 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts 
b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 636d166..9191491 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -169,6 +169,7 @@
 
spi_2: spi@1394 {
status = "okay";
+   cs-gpios = <&gpc1 2 0>;
 
w25x80@0 {
#address-cells = <1>;
@@ -178,7 +179,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpc1 2 0>;
samsung,spi-feedback-delay = <0>;
};
 
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts 
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 8a558b7..204b0de 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -512,6 +512,7 @@
spi_1: spi@1393 {
pinctrl-names = "default";
pinctrl-0 = <&spi1_bus>;
+   cs-gpios = <&gpb 5 0>;
status = "okay";
 
s5c73m3_spi: s5c73m3 {
@@ -519,7 +520,6 @@
spi-max-frequency = <5000>;
reg = <0>;
controller-data {
-   cs-gpio = <&gpb 5 0>;
samsung,spi-feedback-delay = <2>;
};
};
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts 
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a794a70..0c6433a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -316,6 +316,7 @@
};
 
spi_1: spi@12d3 {
+   cs-gpios = <&gpa2 5 0>;
status = "okay";
 
w25q80bw@0 {
@@ -326,7 +327,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpa2 5 0>;
samsung,spi-feedback-delay = <0>;
};
 
-- 
1.7.9.5

--
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/3] spi: s3c64xx: remove a compilation warning with an assignment

[Naveen Krishna Ch]

Hello Sachin,

On 10 June 2014 15:15, Sachin Kamat  wrote:
> Hi Naveen,
>
> On Tue, Jun 10, 2014 at 2:30 PM, Naveen Krishna Chatradhi
>  wrote:
>> This patch returns an integer error value instead of the
>> pointer.
>>
>> "warning: return makes integer from pointer without a cast"
>>
>> Signed-off-by: Naveen Krishna Chatradhi 
>> Cc: Javier Martinez Canillas 
>> Cc: Doug Anderson 
>> ---
>>  drivers/spi/spi-s3c64xx.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 0c6013f..4594dde 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -817,7 +817,7 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>>
>> if (!spi->dev.of_node) {
>> dev_err(&spi->dev, "device node not found\n");
>> -   return ERR_PTR(-EINVAL);
>> +   return -EINVAL;
>
> Isn't this warning introduced by the previous patch (patch1/3)?
> If so then this patch should be squashed into that.
You are right, I've introduced this warning in the 1st patch.
But, realized it only latter. Will spin along with the documentation.
>
> --
> Regards,
> Sachin.



-- 
Shine bright,
(: Nav :)
--
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/3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Ch]

Hello All,


On 10 June 2014 14:30, Naveen Krishna Chatradhi  wrote:
> This patch makes the changes in spi-s3c64xx.c driver to make use of
> "cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
> slaves "controller-data"(child) node.
>
> Signed-off-by: Naveen Krishna Chatradhi 
> Cc: Javier Martinez Canillas 
> Cc: Doug Anderson 
> ---
>  drivers/spi/spi-s3c64xx.c |   56 
> -
>  1 file changed, 35 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 75a5696..0c6013f 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -750,47 +750,56 @@ static int s3c64xx_spi_transfer_one(struct spi_master 
> *master,
>  }
>
>  static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
> -   struct spi_device *spi)
> +   struct spi_device *spi,
> +   struct s3c64xx_spi_csinfo *cs)
>  {
> -   struct s3c64xx_spi_csinfo *cs;
> -   struct device_node *slave_np, *data_np = NULL;
> -   struct s3c64xx_spi_driver_data *sdd;
> +   struct device_node *data_np = NULL;
> u32 fb_delay = 0;
>
> -   sdd = spi_master_get_devdata(spi->master);
> -   slave_np = spi->dev.of_node;
> -   if (!slave_np) {
> -   dev_err(&spi->dev, "device node not found\n");
> +   data_np = of_get_child_by_name(spi->dev.of_node, "controller-data");
> +   if (!data_np) {
> +   dev_err(&spi->dev, "child node 'controller-data' not 
> found\n");
> return ERR_PTR(-EINVAL);
> }
>
> -   data_np = of_get_child_by_name(slave_np, "controller-data");
> -   if (!data_np) {
> -   dev_err(&spi->dev, "child node 'controller-data' not 
> found\n");
> +   of_property_read_u32(data_np, "samsung,spi-feedback-delay", 
> &fb_delay);
> +   cs->fb_delay = fb_delay;
> +   of_node_put(data_np);
> +
> +   return cs;
> +}
> +
> +static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device 
> *spi)
> +{
> +   struct device_node *parent_np = NULL;
> +   struct s3c64xx_spi_driver_data *sdd;
> +   struct s3c64xx_spi_csinfo *cs;
> +
> +   parent_np = of_get_parent(spi->dev.of_node);
> +   if (!parent_np) {
> +   dev_err(&spi->dev, "Parent node not found\n");
> return ERR_PTR(-EINVAL);
> }
>
> +   sdd = spi_master_get_devdata(spi->master);
> +
> cs = kzalloc(sizeof(*cs), GFP_KERNEL);
> if (!cs) {
> -   of_node_put(data_np);
> +   of_node_put(parent_np);
> return ERR_PTR(-ENOMEM);
> }
>
> /* The CS line is asserted/deasserted by the gpio pin */
> if (sdd->cs_gpio)
> -   cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
> +   cs->line = of_get_named_gpio(parent_np, "cs-gpios", 0);
>
> if (!gpio_is_valid(cs->line)) {
> dev_err(&spi->dev, "chip select gpio is not specified or 
> invalid\n");
> -   kfree(cs);
> -   of_node_put(data_np);
> +   of_node_put(parent_np);
> return ERR_PTR(-EINVAL);
> }
>
> -   of_property_read_u32(data_np, "samsung,spi-feedback-delay", 
> &fb_delay);
> -   cs->fb_delay = fb_delay;
> -   of_node_put(data_np);
> -   return cs;
> +   return s3c64xx_get_slave_ctrldata(spi, cs);
>  }
>
>  /*
> @@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
> struct s3c64xx_spi_info *sci;
> int err;
>
> +   if (!spi->dev.of_node) {
> +   dev_err(&spi->dev, "device node not found\n");
> +   return ERR_PTR(-EINVAL);
> +   }
> +
> sdd = spi_master_get_devdata(spi->master);
> if (!cs && spi->dev.of_node) {
> -   cs = s3c64xx_get_slave_ctrldata(spi);
> +   cs = s3c64xx_get_cs_gpios(spi);
> spi->controller_data = cs;
> }
>
> @@ -1077,7 +1091,7 @@ static int s3c64xx_spi_probe(struct platform_device 
> *pdev)
> sdd->sfr_start = mem_res->start;
> sdd->cs_gpio = true;
> if (pdev->dev.of_node) {
> -   if (!of_find_property(pdev->dev.of_node, "cs-gpio", NULL))
> +   if (!of_find_property(pdev->dev.of_node, "cs-gpios", NULL))
> sdd->cs_gpio = false;
>
> ret = of_alias_get_id(pdev->dev.of_node, "spi");
> --
> 1.7.9.5
Forgot to add the DTS documentation.
Will quickly respin. Thanks.
>



-- 
Shine bright,
(: Nav :)
--
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/3] spi: s3c64xx: remove a compilation warning with an assignment

[Sachin Kamat]

Hi Naveen,

On Tue, Jun 10, 2014 at 2:30 PM, Naveen Krishna Chatradhi
 wrote:
> This patch returns an integer error value instead of the
> pointer.
>
> "warning: return makes integer from pointer without a cast"
>
> Signed-off-by: Naveen Krishna Chatradhi 
> Cc: Javier Martinez Canillas 
> Cc: Doug Anderson 
> ---
>  drivers/spi/spi-s3c64xx.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 0c6013f..4594dde 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -817,7 +817,7 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
>
> if (!spi->dev.of_node) {
> dev_err(&spi->dev, "device node not found\n");
> -   return ERR_PTR(-EINVAL);
> +   return -EINVAL;

Isn't this warning introduced by the previous patch (patch1/3)?
If so then this patch should be squashed into that.

-- 
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: Problems booting exynos5420 with >1 CPU

[Lorenzo Pieralisi]

On Tue, Jun 10, 2014 at 05:25:47AM +0100, Nicolas Pitre wrote:
> On Mon, 9 Jun 2014, Lorenzo Pieralisi wrote:
> 
> > I commented on Nico's patch because I did not like how it was
> > implemented (at least remove the CPU PM notifier calls please, because
> > they are not needed).
> 
> OK no problem.  That's easy enough.  I added them to play it safe as a 
> test patch in case some VFP content could be lost somehow by looping 
> back through the CPU init code for example, and needed to be saved.

Ok, thanks.

> > I also said that's the only thing he could do, and I still think 
> > that's not a nice way to use the cpu_suspend API for something it was 
> > not designed for, that's what I wanted to say, period.
> 
> Well... Maybe it wasn't designed for that, but it certainly can be used 
> for that. And with no modifications to the core code, making this 
> solution fairly elegant.  This is not so different from, say, the BPF 
> code being reused for seccomp_filters. BPF wasn't designed for system 
> call filtering, but it happens to work well.

You defined yourself "not a small thing", you know what you are doing
and that's enough for me. Please respect that when I reviewed it I thought
that was a hack. cpu_suspend is being used for many things in the kernel
and consolidating that took a while, please comment the code, that's all I
am asking you.

> > If I am allowed to say something, here is a couple of thoughts.
> > 
> > 1) CCI snoops and DVM enablement are secure operations, to do them in
> >non-secure world this must be overriden in firmware. You can argue,
> >you can think whatever you want, that's a fact. So, to use this
> >code SMP bit in SCTLR and CCI enablement must become non-secure
> >operations. This is a boot requirement for MCPM, right or wrong
> >it is up to platform designers to judge. If CCI and SMP enablement
> >are secure operations, we should not start adding random SMC calls
> >in the kernel, since managing coherency in the kernel would become
> >problematic, with lots of platform quirks. We do not want that to
> >happen, and I think we all agree on this.
> 
> One could certainly question the need for so many controls handled in 
> secure world.  But that is not the point.
> 
> Here we're talking about MCPM.  That implies the kernel has control over 
> SCTLR.SMP and the CCI.  If those things aren't under the kernel's 
> control, then MCPM is of no use to you.

ACTLR.SMP for the sake of precision and it was my typo, sorry. That's
all I wanted to read, so nothing to add.

> Therefore, if you do want to use MCPM, then this implies the kernel has 
> access to the CCI. And if it has access to it, then it should turn it on 
> by itself in all cases to be consistent, not only in half of the cases.

I agree.

> > 2) (1) must be documented.
> 
> Absolutely.  But let's be coherent in the implementation so the 
> documentation is as simple as it can be.

Ditto.

> > 3) When I talked about consequences for CPUidle (implicit), I was referring
> >to all sort of hacks we had to come up to bring devices like SPC
> >(remember ? I remember very very well unfortunately for me), or whatever
> >power controller up in the kernel early, too early to fit in any
> >existing kernel device framework. There is still no solution to that, and
> >the only way that code can exist is in mach- code. Right or wrong,
> >that's a second fact and in my opinion that's not nice for the ARM
> >kernel.
> 
> I disagree.  This can perfectly be turned into driver code. If we need 
> it too early for existing kernel device framework to handle this 
> properly, then the solution is to extend the existing framework or 
> create another one specially for that purpose.  This may not be obvious 
> when TC2 is the first/only platform in that situation, but if more 
> platforms have the same need then it'll be easier to abstract 
> commonalities into a framework.
> 
> Saying that no framework exists today or/and upstream maintainers are 
> being difficult is _not_ a reason for throwing your hands up and e.g. 
> shoving all this code into firmware instead.

You have a point, as long as we are all aware and we do not forget this
is a major problem, not a minor one. I do want to see a consolidate
story for CPUidle for ARM and this bullet is definitely part of the
picture. On a side note, you made me smile, it sounded like I wanted
to bury SPC code in firmware or anywhere else as long as it is not in the
kernel, which in a way is a true statement since I abhor that code =)

> > 4) When I am talking about firmware I am talking about sequences that
> >are very close to HW (disabling C bit, cleaning caches, exiting
> >coherency). Erratas notwithstanding, they are being standardized at
> >ARM the best we can. They might even end up being implemented in HW
> >in the not so far future. I understand they are tricky, I understand
> >they take lots of time to implement t

[PATCH 0/3] spi: s3c64xx: use "cs-gpios" in spi node instead of "cs-gpio"

[Naveen Krishna Chatradhi]

Currently, spi-s3c64xx.c needs "cs-gpio" chip select GPIO to be
defined under "controller-data" node under each slave node.

&spi_x {
cs-gpios <>;
...
slave_node {

controller-data {
cs-gpio = <>;
...
};
...
};
...
};

Where as, SPI core and many other drivers uses "cs-gpios" for
from device tree node.

Hence, make changes in spi-s3c64xx.c driver to make use of
"cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
slaves "controller-data"(child) node.

Also, fixes a compilation warning and corrects the DTS nodes for
Exynos4210 based SMDKv310, Exynos4412 based Trats2, Exynos5250 based
SMDK5250 boards.

Naveen Krishna Chatradhi (3):
based on for-next branch of spi.git
  spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"
  spi: s3c64xx: remove a compilation warning with an assignment

based on for-next branch of linuxsamsung.git
  ARM: DTS: move "cs-gpio" from "controller-data" to under spi node

 arch/arm/boot/dts/exynos4210-smdkv310.dts |2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts   |2 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |2 +-
 drivers/spi/spi-s3c64xx.c |   56 ++---
 4 files changed, 38 insertions(+), 24 deletions(-)

-- 
1.7.9.5

--
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 1/3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"

[Naveen Krishna Chatradhi]

This patch makes the changes in spi-s3c64xx.c driver to make use of
"cs-gpios" from SPI node(parent) instead of "cs-gpio" defined in
slaves "controller-data"(child) node.

Signed-off-by: Naveen Krishna Chatradhi 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
---
 drivers/spi/spi-s3c64xx.c |   56 -
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 75a5696..0c6013f 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -750,47 +750,56 @@ static int s3c64xx_spi_transfer_one(struct spi_master 
*master,
 }
 
 static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
-   struct spi_device *spi)
+   struct spi_device *spi,
+   struct s3c64xx_spi_csinfo *cs)
 {
-   struct s3c64xx_spi_csinfo *cs;
-   struct device_node *slave_np, *data_np = NULL;
-   struct s3c64xx_spi_driver_data *sdd;
+   struct device_node *data_np = NULL;
u32 fb_delay = 0;
 
-   sdd = spi_master_get_devdata(spi->master);
-   slave_np = spi->dev.of_node;
-   if (!slave_np) {
-   dev_err(&spi->dev, "device node not found\n");
+   data_np = of_get_child_by_name(spi->dev.of_node, "controller-data");
+   if (!data_np) {
+   dev_err(&spi->dev, "child node 'controller-data' not found\n");
return ERR_PTR(-EINVAL);
}
 
-   data_np = of_get_child_by_name(slave_np, "controller-data");
-   if (!data_np) {
-   dev_err(&spi->dev, "child node 'controller-data' not found\n");
+   of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
+   cs->fb_delay = fb_delay;
+   of_node_put(data_np);
+
+   return cs;
+}
+
+static struct s3c64xx_spi_csinfo *s3c64xx_get_cs_gpios(struct spi_device *spi)
+{
+   struct device_node *parent_np = NULL;
+   struct s3c64xx_spi_driver_data *sdd;
+   struct s3c64xx_spi_csinfo *cs;
+
+   parent_np = of_get_parent(spi->dev.of_node);
+   if (!parent_np) {
+   dev_err(&spi->dev, "Parent node not found\n");
return ERR_PTR(-EINVAL);
}
 
+   sdd = spi_master_get_devdata(spi->master);
+
cs = kzalloc(sizeof(*cs), GFP_KERNEL);
if (!cs) {
-   of_node_put(data_np);
+   of_node_put(parent_np);
return ERR_PTR(-ENOMEM);
}
 
/* The CS line is asserted/deasserted by the gpio pin */
if (sdd->cs_gpio)
-   cs->line = of_get_named_gpio(data_np, "cs-gpio", 0);
+   cs->line = of_get_named_gpio(parent_np, "cs-gpios", 0);
 
if (!gpio_is_valid(cs->line)) {
dev_err(&spi->dev, "chip select gpio is not specified or 
invalid\n");
-   kfree(cs);
-   of_node_put(data_np);
+   of_node_put(parent_np);
return ERR_PTR(-EINVAL);
}
 
-   of_property_read_u32(data_np, "samsung,spi-feedback-delay", &fb_delay);
-   cs->fb_delay = fb_delay;
-   of_node_put(data_np);
-   return cs;
+   return s3c64xx_get_slave_ctrldata(spi, cs);
 }
 
 /*
@@ -806,9 +815,14 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
struct s3c64xx_spi_info *sci;
int err;
 
+   if (!spi->dev.of_node) {
+   dev_err(&spi->dev, "device node not found\n");
+   return ERR_PTR(-EINVAL);
+   }
+
sdd = spi_master_get_devdata(spi->master);
if (!cs && spi->dev.of_node) {
-   cs = s3c64xx_get_slave_ctrldata(spi);
+   cs = s3c64xx_get_cs_gpios(spi);
spi->controller_data = cs;
}
 
@@ -1077,7 +1091,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)
sdd->sfr_start = mem_res->start;
sdd->cs_gpio = true;
if (pdev->dev.of_node) {
-   if (!of_find_property(pdev->dev.of_node, "cs-gpio", NULL))
+   if (!of_find_property(pdev->dev.of_node, "cs-gpios", NULL))
sdd->cs_gpio = false;
 
ret = of_alias_get_id(pdev->dev.of_node, "spi");
-- 
1.7.9.5

--
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 2/3] spi: s3c64xx: remove a compilation warning with an assignment

[Naveen Krishna Chatradhi]

This patch returns an integer error value instead of the
pointer.

"warning: return makes integer from pointer without a cast"

Signed-off-by: Naveen Krishna Chatradhi 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
---
 drivers/spi/spi-s3c64xx.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 0c6013f..4594dde 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -817,7 +817,7 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 
if (!spi->dev.of_node) {
dev_err(&spi->dev, "device node not found\n");
-   return ERR_PTR(-EINVAL);
+   return -EINVAL;
}
 
sdd = spi_master_get_devdata(spi->master);
-- 
1.7.9.5

--
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 3/3] ARM: DTS: move "cs-gpio" from "controller-data" to under spi node

[Naveen Krishna Chatradhi]

This patch moves the "cs-gpio" field from "controller-data" child
node to under the spi device node.

Respective changes are preposed to spi-s3c64xx.c driver.

Signed-off-by: Naveen Krishna Chatradhi 
Cc: Javier Martinez Canillas 
Cc: Doug Anderson 
---
 arch/arm/boot/dts/exynos4210-smdkv310.dts |2 +-
 arch/arm/boot/dts/exynos4412-trats2.dts   |2 +-
 arch/arm/boot/dts/exynos5250-smdk5250.dts |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-smdkv310.dts 
b/arch/arm/boot/dts/exynos4210-smdkv310.dts
index 636d166..9191491 100644
--- a/arch/arm/boot/dts/exynos4210-smdkv310.dts
+++ b/arch/arm/boot/dts/exynos4210-smdkv310.dts
@@ -169,6 +169,7 @@
 
spi_2: spi@1394 {
status = "okay";
+   cs-gpios = <&gpc1 2 0>;
 
w25x80@0 {
#address-cells = <1>;
@@ -178,7 +179,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpc1 2 0>;
samsung,spi-feedback-delay = <0>;
};
 
diff --git a/arch/arm/boot/dts/exynos4412-trats2.dts 
b/arch/arm/boot/dts/exynos4412-trats2.dts
index 8a558b7..204b0de 100644
--- a/arch/arm/boot/dts/exynos4412-trats2.dts
+++ b/arch/arm/boot/dts/exynos4412-trats2.dts
@@ -512,6 +512,7 @@
spi_1: spi@1393 {
pinctrl-names = "default";
pinctrl-0 = <&spi1_bus>;
+   cs-gpios = <&gpb 5 0>;
status = "okay";
 
s5c73m3_spi: s5c73m3 {
@@ -519,7 +520,6 @@
spi-max-frequency = <5000>;
reg = <0>;
controller-data {
-   cs-gpio = <&gpb 5 0>;
samsung,spi-feedback-delay = <2>;
};
};
diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts 
b/arch/arm/boot/dts/exynos5250-smdk5250.dts
index a794a70..0c6433a 100644
--- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
+++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
@@ -316,6 +316,7 @@
};
 
spi_1: spi@12d3 {
+   cs-gpios = <&gpa2 5 0>;
status = "okay";
 
w25q80bw@0 {
@@ -326,7 +327,6 @@
spi-max-frequency = <100>;
 
controller-data {
-   cs-gpio = <&gpa2 5 0>;
samsung,spi-feedback-delay = <0>;
};
 
-- 
1.7.9.5

--
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 CRITICAL] ARM: s3c64xx: dt: Fix boot failure due to double clock initialization

[Olof Johansson]

On Tue, Dec 17, 2013 at 11:12 AM, Olof Johansson  wrote:
> On Tue, Dec 17, 2013 at 7:14 AM, Charles Keepax
>  wrote:
>> On Mon, Dec 16, 2013 at 09:09:15PM +, Mark Brown wrote:
>>> On Sat, Dec 14, 2013 at 04:41:06AM -0800, Tomasz Figa wrote:
>>>
>>> >  - Wolfson Cragganmore 6410 - Mark Brown (copied) is using it for some
>>> >testing of audio codec AFAIK, but I'm not sure if you can get that
>>> >anywhere.
>>>
>>> Essentially no, though you could try asking nicely.  It's not suitable
>>> for a test farm anyway without a SD mux as the only bootloader supported
>>> is Qi which only boots from flash.
>>
>> Yeah, we don't have that many spare at the moment and whilst we
>> do use them fairly regularly we are not produce new boards
>> anymore.
>
> Yeah, sounds unlikely to be a good fit.
>
> Something like the tiny6410's would be useful. They seem to mostly be
> available with LCD panels though, which ups the price a bit. I'll keep
> an eye out for one.

FWIW, I came across a store at Guang Hua Digital Plaza today that had
the tiny6410+SDK board on display, and ended up buying one. It'll take
me a little while to wire it up for testing.

Seems like there's no upstream u-boot support for the board, so
getting something onto it that can network boot seems to be a long
shot. We'll see what I can cobble together.


-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] ARM: EXYNOS: mcpm: Don't rely on firmware's secondary_cpu_start

[Chander Kashyap]

On 10 June 2014 04:08, Lorenzo Pieralisi  wrote:
> On Mon, Jun 09, 2014 at 06:03:31PM +0100, Doug Anderson wrote:
>
> [...]
>
>> Cold boot and resume from suspend are detected via various special
>> flags in various special locations.  Resume from suspend looks at
>> INFORM1 (0x10048004) for flags.  This register is 0 during a cold boot
>> and has special values set by the kernel at resume time.
>>
>> It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help
>> tell initial cold boot and secondary CPU bringup.
>
> Ok, thanks a lot. It looks like firmware paths should be ready to
> detect cold vs warm boot, and hopefully do not rely on a specific
> MPIDR to come up first out of power states.
>
>> > I am asking to check if on this platform CPUidle (where the notion of
>> > primary CPU disappears) has a chance to run properly.
>>
>> I believe it should be possible, but we don't have CPUidle implemented
>> in our current system.  Abhilash may be able to comment more.
>

Cpuidle is implemented for exynos5420, and is tested on chromebook.


> I am interested in more insights, that's very helpful thanks.
>
>> > Probably CPUidle won't attain idle states where IRAM content is lost, but I
>> > am still worried about the primary vs secondaries firmware boot behaviour.
>>
>> I don't think iRAM can be turned off for CPUidle.

Yes thats true.
>
> It might be added a system state but I doubt that too and if you are
> relying on registers for jump addresses that's not even a problem in
> the first place.
>
>> > What happens on reboot from suspend to RAM (or to put it differently,
>> > what does secure firmware do on reboot from suspend to RAM - in
>> > particular how is the "jump" address to bootloader/kernel set ?)
>>
>> Should be described above now.
>
> Thank you very much.
>
> Lorenzo
>
> --
> 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,
Chander Kashyap
--
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 0/5] Add Maxim 77802 PMIC support

[Javier Martinez Canillas]

Hello Krzysztof,

> On 10/06/2014, at 09:32, Krzysztof Kozlowski  wrote:
> 
>> On pon, 2014-06-09 at 09:04 -0700, Doug Anderson wrote:
>> Krzystof,
>> 
>> On Mon, Jun 9, 2014 at 3:16 AM, Krzysztof Kozlowski
>>  wrote:
>>> On pon, 2014-06-09 at 11:37 +0200, Javier Martinez Canillas wrote:
 MAX77802 is a PMIC that contains 10 high efficiency Buck regulators,
 32 Low-dropout (LDO) regulators, two 32kHz buffered clock outputs,
 a Real-Time-Clock (RTC) and a I2C interface to program the individual
 regulators, clocks and the RTC.
 
 This series are based on drivers added by Simon Glass to the Chrome OS
 kernel and adds support for the Maxim 77802 Power Management IC, their
 regulators, clocks, RTC and I2C interface. It is composed of patches:
 
 [PATCH 1/5] mfd: Add driver for Maxim 77802 Power Management IC
 [PATCH 2/5] regulator: Add driver for Maxim 77802 PMIC regulators
 [PATCH 3/5] clk: Add driver for Maxim 77802 PMIC clocks
 [PATCH 4/5] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock
 [PATCH 5/5] ARM: dts: Add max77802 device node for exynos5420-peach-pit
 
 Patches 1-4 add support for the different devices and Patch 5 enables
 the MAX77802 PMIC on the Exynos5420 based Peach pit board.
>>> 
>>> 
>>> Hi,
>>> 
>>> The main mfd, mfd irq, clk and rtc drivers look very similar to max77686
>>> drivers. I haven't checked other Maxim drivers but I think there will be
>>> a lot of similarities with them also. It is almost common for Maxim
>>> chipsets to share components between each other.
>>> 
>>> I think there is no need in duplicating all that stuff once again in new
>>> driver for another Maxim-almost-the-same-as-others-XYZ chipset. Just
>>> merge it with max77686 (or other better candidate).
>>> 
>>> The only difference is in regulator driver. I am not sure whether this
>>> is a result of differences in chip or differences in driver design.
>> 
>> Yes, we thought the same thing when we added support for the max77802
>> in the ChromeOS tree.  Unfortunately it didn't work out half as well
>> as we thought it would.  When Javier was asking advice about sending
>> things upstream we suggested that perhaps he should split the two up.
>> 
>> 
>> You can see the result of the combined driver the ChromeOS tree (the
>> code there is older, probably misnamed as max77xxx, and doesn't have
>> the proper clock pieces, but you can get the gist):
>> 
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/max77xxx-regulator.c
>> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/rtc/rtc-max77xxx.c
>> 
>> 
>> Specific problems that made it ugly to have a combined driver:
>> 
>> * The RTC has many subtle differences between the 77686 and 77802.
>> They expanded it to handle a 200 year timeframe instead of 100 and
>> that meant that they had to shuffle the bits around everywhere.  They
>> also moved it to have the same i2c address as the main PMIC so all
>> addresses are different (see max77686_map in the RTC link above).
> 
> The difference in RTC registers seems the biggest but it can be solved
> in readable manner. I see other differences but there aren't many. It
> just hurts seeing so much code duplication:
> $ sed -e 's/max77686/max77802/g' -e 's/MAX77686/MAX77802/g' \
>  -i drivers/rtc/rtc-max77686.c
> $ diff -ubB drivers/rtc/rtc-max77686.c drivers/rtc/rtc-max77802.c
> 
> The combined RTC driver from ChromeOS seems fine to me... but I do not
> insist.
> 
>> * The regulator itself has similar concepts between the two, but the
>> list of bucks / ldos and how they behave is quite different.  Trying
>> to understand the complex tables in
>> 
>> was not easy.
>> 
>> 
>> If we really need to write a single driver it certainly can be done,
>> but please look at the above to be sure this is what you want.
> 
> Sure, I don't stick to the idea of one merged driver where this
> increases code size and complexity. I see your point that merging
> regulator drivers won't bring benefits but please:
> $ sed -e 's/max77686/max77802/g' -e 's/MAX77686/MAX77802/g' \
>  -i drivers/clk/clk-max77686.c
> $ diff -ubB drivers/clk/clk-max77686.c drivers/clk/clk-max77802.c
> 

I agree that there is too much duplicated code between those two and others 
Maxim PMIC drivers.

I also agree that at some point we have to stop duplicating code and clean up 
all this.

So, I think that instead of trying to make a single driver support two similar 
but different PMIC I can factor out the generic code in a max-core driver so 
the PMIC specific code is small and well contained.

I'll work on that and post a v2.

Thanks a lot for your feedback and best regards,

Javier

> The difference in number of clocks (2 vs 3) is not an obstacle here.
> 
> The same applies to main MFD driver and IRQ cod

Re: [PATCH 0/5] Add Maxim 77802 PMIC support

[Krzysztof Kozlowski]

On wto, 2014-06-10 at 00:55 +0200, Javier Martinez Canillas wrote:
> Hello Krzystof,
> 
> Thanks a lot for your feedback.
> 
> On 06/09/2014 06:04 PM, Doug Anderson wrote:
> > Krzystof,
> > 
> > On Mon, Jun 9, 2014 at 3:16 AM, Krzysztof Kozlowski
> >  wrote:
> >> On pon, 2014-06-09 at 11:37 +0200, Javier Martinez Canillas wrote:
> >>> MAX77802 is a PMIC that contains 10 high efficiency Buck regulators,
> >>> 32 Low-dropout (LDO) regulators, two 32kHz buffered clock outputs,
> >>> a Real-Time-Clock (RTC) and a I2C interface to program the individual
> >>> regulators, clocks and the RTC.
> >>>
> >>> This series are based on drivers added by Simon Glass to the Chrome OS
> >>> kernel and adds support for the Maxim 77802 Power Management IC, their
> >>> regulators, clocks, RTC and I2C interface. It is composed of patches:
> >>>
> >>> [PATCH 1/5] mfd: Add driver for Maxim 77802 Power Management IC
> >>> [PATCH 2/5] regulator: Add driver for Maxim 77802 PMIC regulators
> >>> [PATCH 3/5] clk: Add driver for Maxim 77802 PMIC clocks
> >>> [PATCH 4/5] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock
> >>> [PATCH 5/5] ARM: dts: Add max77802 device node for exynos5420-peach-pit
> >>>
> >>> Patches 1-4 add support for the different devices and Patch 5 enables
> >>> the MAX77802 PMIC on the Exynos5420 based Peach pit board.
> >>
> >>
> >> Hi,
> >>
> >> The main mfd, mfd irq, clk and rtc drivers look very similar to max77686
> >> drivers. I haven't checked other Maxim drivers but I think there will be
> >> a lot of similarities with them also. It is almost common for Maxim
> >> chipsets to share components between each other.
> >>
> >> I think there is no need in duplicating all that stuff once again in new
> >> driver for another Maxim-almost-the-same-as-others-XYZ chipset. Just
> >> merge it with max77686 (or other better candidate).
> >>
> >> The only difference is in regulator driver. I am not sure whether this
> >> is a result of differences in chip or differences in driver design.
> > 
> > Yes, we thought the same thing when we added support for the max77802
> > in the ChromeOS tree.  Unfortunately it didn't work out half as well
> > as we thought it would.  When Javier was asking advice about sending
> > things upstream we suggested that perhaps he should split the two up.
> > 
> > 
> > You can see the result of the combined driver the ChromeOS tree (the
> > code there is older, probably misnamed as max77xxx, and doesn't have
> > the proper clock pieces, but you can get the gist):
> > 
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/max77xxx-regulator.c
> > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/rtc/rtc-max77xxx.c
> > 
> > 
> > Specific problems that made it ugly to have a combined driver:
> > 
> > * The RTC has many subtle differences between the 77686 and 77802.
> > They expanded it to handle a 200 year timeframe instead of 100 and
> > that meant that they had to shuffle the bits around everywhere.  They
> > also moved it to have the same i2c address as the main PMIC so all
> > addresses are different (see max77686_map in the RTC link above).
> >
> > * The regulator itself has similar concepts between the two, but the
> > list of bucks / ldos and how they behave is quite different.  Trying
> > to understand the complex tables in
> > 
> > was not easy.
> > 
> > 
> 
> There are other differences that were not mentioned:
> 
> - The max77802 uses a single register to enable RTC alarm while max77686 uses 
> 1
> bit from a set of registers.

But still this will be one additional 'if' statement in one common
code...

> - Each chip has some regulators that are not available and you have to take 
> care
> of those exceptions (e.g: LDO16, LDO22 and LD31 on max77802).

Missing/new regulator does not look like a problem to me in combined
driver from ChromeOS. The main problem with combined driver for
regulators is that you still need to provide all the
regulator_ops/regulator_desc for each of the chipsets. Thus the combined
driver is almost as big as two drivers.

> - The max77802 has 2 clocks outputs while the max77686 has three.

This leads to one exception in combined clock driver. This will still
reduce LOC.

> So, as Doug said there are many differences on these two chips besides the
> regulators. It's true that these two drivers share a lot of the structure and
> there is code duplication (this applies to most maxim drivers btw) but I have 
> my
> doubts that the combined approach will make the code more easy to maintain.
> 
> The biggest problem is the i2c addresses space being different so you need an
> indirection level to access registers and have duplicated registers definition
> for each chip.

Yep, I agree. I had the same problem with S5M/S2M RTC driver and I am
not quite sure that I've chose

Re: [PATCH 0/5] Add Maxim 77802 PMIC support

[Krzysztof Kozlowski]

On pon, 2014-06-09 at 09:04 -0700, Doug Anderson wrote:
> Krzystof,
> 
> On Mon, Jun 9, 2014 at 3:16 AM, Krzysztof Kozlowski
>  wrote:
> > On pon, 2014-06-09 at 11:37 +0200, Javier Martinez Canillas wrote:
> >> MAX77802 is a PMIC that contains 10 high efficiency Buck regulators,
> >> 32 Low-dropout (LDO) regulators, two 32kHz buffered clock outputs,
> >> a Real-Time-Clock (RTC) and a I2C interface to program the individual
> >> regulators, clocks and the RTC.
> >>
> >> This series are based on drivers added by Simon Glass to the Chrome OS
> >> kernel and adds support for the Maxim 77802 Power Management IC, their
> >> regulators, clocks, RTC and I2C interface. It is composed of patches:
> >>
> >> [PATCH 1/5] mfd: Add driver for Maxim 77802 Power Management IC
> >> [PATCH 2/5] regulator: Add driver for Maxim 77802 PMIC regulators
> >> [PATCH 3/5] clk: Add driver for Maxim 77802 PMIC clocks
> >> [PATCH 4/5] rtc: Add driver for Maxim 77802 PMIC Real-Time-Clock
> >> [PATCH 5/5] ARM: dts: Add max77802 device node for exynos5420-peach-pit
> >>
> >> Patches 1-4 add support for the different devices and Patch 5 enables
> >> the MAX77802 PMIC on the Exynos5420 based Peach pit board.
> >
> >
> > Hi,
> >
> > The main mfd, mfd irq, clk and rtc drivers look very similar to max77686
> > drivers. I haven't checked other Maxim drivers but I think there will be
> > a lot of similarities with them also. It is almost common for Maxim
> > chipsets to share components between each other.
> >
> > I think there is no need in duplicating all that stuff once again in new
> > driver for another Maxim-almost-the-same-as-others-XYZ chipset. Just
> > merge it with max77686 (or other better candidate).
> >
> > The only difference is in regulator driver. I am not sure whether this
> > is a result of differences in chip or differences in driver design.
> 
> Yes, we thought the same thing when we added support for the max77802
> in the ChromeOS tree.  Unfortunately it didn't work out half as well
> as we thought it would.  When Javier was asking advice about sending
> things upstream we suggested that perhaps he should split the two up.
> 
> 
> You can see the result of the combined driver the ChromeOS tree (the
> code there is older, probably misnamed as max77xxx, and doesn't have
> the proper clock pieces, but you can get the gist):
> 
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/regulator/max77xxx-regulator.c
> https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/rtc/rtc-max77xxx.c
> 
> 
> Specific problems that made it ugly to have a combined driver:
> 
> * The RTC has many subtle differences between the 77686 and 77802.
> They expanded it to handle a 200 year timeframe instead of 100 and
> that meant that they had to shuffle the bits around everywhere.  They
> also moved it to have the same i2c address as the main PMIC so all
> addresses are different (see max77686_map in the RTC link above).

The difference in RTC registers seems the biggest but it can be solved
in readable manner. I see other differences but there aren't many. It
just hurts seeing so much code duplication:
$ sed -e 's/max77686/max77802/g' -e 's/MAX77686/MAX77802/g' \
  -i drivers/rtc/rtc-max77686.c
$ diff -ubB drivers/rtc/rtc-max77686.c drivers/rtc/rtc-max77802.c

The combined RTC driver from ChromeOS seems fine to me... but I do not
insist.

> * The regulator itself has similar concepts between the two, but the
> list of bucks / ldos and how they behave is quite different.  Trying
> to understand the complex tables in
> 
> was not easy.
> 
> 
> If we really need to write a single driver it certainly can be done,
> but please look at the above to be sure this is what you want.

Sure, I don't stick to the idea of one merged driver where this
increases code size and complexity. I see your point that merging
regulator drivers won't bring benefits but please:
$ sed -e 's/max77686/max77802/g' -e 's/MAX77686/MAX77802/g' \
  -i drivers/clk/clk-max77686.c
$ diff -ubB drivers/clk/clk-max77686.c drivers/clk/clk-max77802.c

The difference in number of clocks (2 vs 3) is not an obstacle here.

The same applies to main MFD driver and IRQ code. However MAX77686
doesn't use regmap_irq_chip so it needs changes before merging the IRQ
code into one piece.

Best regards,
Krzysztof

> 
> NOTE: it's possible that things could be more sane with more driver
> redesign, possibly making things more data driven.  The thing that
> would be really nice to do would be to avoid all of the crazy
> "regulator_zzz_desc_zzz" macros, maybe?  I'd have to actually try
> doing it to be sure it's cleaner, though...
> 
> 
> -Doug

--
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/ma