Re: [PATCH] ARM: dts: remove display power domain for exynos5420

[Rahul Sharma]

Hi Andrej,

On 18 June 2014 11:46, Andrzej Hajda  wrote:
> On 06/17/2014 07:49 AM, Rahul Sharma wrote:
>> Hi All,
>>
>> Please review this patch.
>>
>> Regards,
>> Rahul Sharma
>>
>> On 9 June 2014 16:58, Rahul Sharma  wrote:
>>> Display domain is removed due to instability issues. Explaining
>>> the problem below:
>>>
>>> exynos_init_late triggers the pm_genpd_poweroff_unused which
>>> powers off the unused power domains. This call hits before
>>> the trigger to deferred probes.
>>>
>>> DRM DP Panel defers the probe due to supply get failure. By the
>>> time, deferred probe is scheduled again, Display Power Domain is
>>> powered off by pm_genpd_poweroff_unused.
>>>
>>> FIMD and DP drivers are accessing registers during Probe and Bind
>>> callbacks. If display domain is enabled/disabled around register
>>> accesses, display domain gets unstable and we are getting Power
>>> Domain Disable fail notification. Increasing the Timeout also
>>> didn't help.
>
> As I understand the problem is that fimd and dp drivers access hw
> registers without enabling power domain. So the proper solution is to
> fix these drivers.

That is also a problem but I fixed those accesses in my local kernel before
hitting this issue. If we do register accesses in FIMD/DP probe/bind we
observes "Prefetch abort" exception. But here the problem is that 'DP
domain disable' starts failing if we enable/disable multiple times.

>
> Btw. there are already patches removing hw access from probe/bind of
> fimd. I guess removing also hw access from dp probe/bind could be a good
> solution.

Please let me know the links for posted patches. I will test with those patches.

Regards,
Rahul Sharma

>
> Regards
> Andrzej
>
>>>
>>> Signed-off-by: Rahul Sharma 
>>> ---
>>> based on Kukjin's for-next branch.
>>>
>>>  arch/arm/boot/dts/exynos5420.dtsi |6 --
>>>  1 file changed, 6 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
>>> b/arch/arm/boot/dts/exynos5420.dtsi
>>> index e385322..3d528cf 100644
>>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>>> @@ -262,11 +262,6 @@
>>> reg = <0x10044060 0x20>;
>>> };
>>>
>>> -   disp_pd: power-domain@100440C0 {
>>> -   compatible = "samsung,exynos4210-pd";
>>> -   reg = <0x100440C0 0x20>;
>>> -   };
>>> -
>>> msc_pd: power-domain@10044120 {
>>> compatible = "samsung,exynos4210-pd";
>>> reg = <0x10044120 0x20>;
>>> @@ -518,7 +513,6 @@
>>> };
>>>
>>> fimd: fimd@1440 {
>>> -   samsung,power-domain = <&disp_pd>;
>>> clocks = <&clock CLK_SCLK_FIMD1>, <&clock CLK_FIMD1>;
>>> clock-names = "sclk_fimd", "fimd";
>>> };
>>> --
>>> 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: [PATCHv4 3/4] iio: devicetree: Add DT binding documentation for Exynos3250 ADC

[Chanwoo Choi]

Hi Naveen,

On 06/18/2014 03:12 PM, Naveen Krishna Ch wrote:
> Hello Chanwoo,
> 
> On 18 June 2014 07:51, Chanwoo Choi  wrote:
>> This patch add DT binding documentation for Exynos3250 ADC IP. Exynos3250 has
>> special clock ('sclk_tsadc') for ADC which provide clock to internal ADC.
>>
>> Signed-off-by: Chanwoo Choi 
>> Acked-by: Kyungmin Park 
> 
> Changes look good to me.
> Reviewed-by: Naveen Krishna Chatradhi 

Thanks for your review.

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: [PATCH] ARM: dts: remove display power domain for exynos5420

[Andrzej Hajda]

On 06/17/2014 07:49 AM, Rahul Sharma wrote:
> Hi All,
> 
> Please review this patch.
> 
> Regards,
> Rahul Sharma
> 
> On 9 June 2014 16:58, Rahul Sharma  wrote:
>> Display domain is removed due to instability issues. Explaining
>> the problem below:
>>
>> exynos_init_late triggers the pm_genpd_poweroff_unused which
>> powers off the unused power domains. This call hits before
>> the trigger to deferred probes.
>>
>> DRM DP Panel defers the probe due to supply get failure. By the
>> time, deferred probe is scheduled again, Display Power Domain is
>> powered off by pm_genpd_poweroff_unused.
>>
>> FIMD and DP drivers are accessing registers during Probe and Bind
>> callbacks. If display domain is enabled/disabled around register
>> accesses, display domain gets unstable and we are getting Power
>> Domain Disable fail notification. Increasing the Timeout also
>> didn't help.

As I understand the problem is that fimd and dp drivers access hw
registers without enabling power domain. So the proper solution is to
fix these drivers.

Btw. there are already patches removing hw access from probe/bind of
fimd. I guess removing also hw access from dp probe/bind could be a good
solution.

Regards
Andrzej

>>
>> Signed-off-by: Rahul Sharma 
>> ---
>> based on Kukjin's for-next branch.
>>
>>  arch/arm/boot/dts/exynos5420.dtsi |6 --
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos5420.dtsi 
>> b/arch/arm/boot/dts/exynos5420.dtsi
>> index e385322..3d528cf 100644
>> --- a/arch/arm/boot/dts/exynos5420.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420.dtsi
>> @@ -262,11 +262,6 @@
>> reg = <0x10044060 0x20>;
>> };
>>
>> -   disp_pd: power-domain@100440C0 {
>> -   compatible = "samsung,exynos4210-pd";
>> -   reg = <0x100440C0 0x20>;
>> -   };
>> -
>> msc_pd: power-domain@10044120 {
>> compatible = "samsung,exynos4210-pd";
>> reg = <0x10044120 0x20>;
>> @@ -518,7 +513,6 @@
>> };
>>
>> fimd: fimd@1440 {
>> -   samsung,power-domain = <&disp_pd>;
>> clocks = <&clock CLK_SCLK_FIMD1>, <&clock CLK_FIMD1>;
>> clock-names = "sclk_fimd", "fimd";
>> };
>> --
>> 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: [PATCHv4 3/4] iio: devicetree: Add DT binding documentation for Exynos3250 ADC

[Naveen Krishna Ch]

Hello Chanwoo,

On 18 June 2014 07:51, Chanwoo Choi  wrote:
> This patch add DT binding documentation for Exynos3250 ADC IP. Exynos3250 has
> special clock ('sclk_tsadc') for ADC which provide clock to internal ADC.
>
> Signed-off-by: Chanwoo Choi 
> Acked-by: Kyungmin Park 

Changes look good to me.
Reviewed-by: Naveen Krishna Chatradhi 

> ---
>  .../devicetree/bindings/arm/samsung/exynos-adc.txt   | 20 
> 
>  1 file changed, 20 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
> b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index 5d49f2b..3a5af82 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -14,6 +14,8 @@ Required properties:
> for exynos4412/5250 controllers.
> Must be "samsung,exynos-adc-v2" for
> future controllers.
> +   Must be "samsung,exynos3250-adc-v2" for
> +   for exynos3250 controllers.
>  - reg: Contains ADC register address range (base address and
> length) and the address of the phy enable register.
>  - interrupts:  Contains the interrupt information for the timer. The
> @@ -21,7 +23,11 @@ Required properties:
> the Samsung device uses.
>  - #io-channel-cells = <1>; As ADC has multiple outputs
>  - clocks   From common clock binding: handle to adc clock.
> +   From common clock binding: handle to sclk_tsadc clock
> +   if using Exynos3250.
>  - clock-names  From common clock binding: Shall be "adc".
> +   From common clock binding: Shall be "sclk_tsadc"
> +   if using Exynos3250.
>  - vdd-supply   VDD input supply.
>
>  Note: child nodes can be added for auto probing from device tree.
> @@ -41,6 +47,20 @@ adc: adc@12D1 {
> vdd-supply = <&buck5_reg>;
>  };
>
> +Example: adding device info in dtsi file for Exynos3250 with additional sclk
> +
> +adc: adc@126C {
> +   compatible = "samsung,exynos3250-adc-v2";
> +   reg = <0x126C 0x100>, <0x10020718 0x4>;
> +   interrupts = <0 137 0>;
> +   #io-channel-cells = <1>;
> +   io-channel-ranges;
> +
> +   clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
> +   clock-names = "adc", "sclk_adc";
> +
> +   vdd-supply = <&buck5_reg>;
> +};
>
>  Example: Adding child nodes in dts file
>
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
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: [PATCHv4 1/4] iio: adc: exynos_adc: Add exynos_adc_ops structure to improve readability

[Chanwoo Choi]

Hi Naveen,

On 06/18/2014 02:27 PM, Naveen Krishna Ch wrote:
> Hello Chanwoo,
> 
> On 18 June 2014 07:50, Chanwoo Choi  wrote:
>> This patchset add 'exynos_adc_ops' structure which includes some functions
>> to control ADC operation according to ADC version (v1 or v2).
>>
>> Signed-off-by: Chanwoo Choi 
>> Acked-by: Kyungmin Park 
> 
> This is a good piece of change, Thanks.
> 
> Reviewed-by: Naveen Krishna Chatradhi 

Thanks for your review.

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: [PATCHv4 1/4] iio: adc: exynos_adc: Add exynos_adc_ops structure to improve readability

[Naveen Krishna Ch]

Hello Chanwoo,

On 18 June 2014 07:50, Chanwoo Choi  wrote:
> This patchset add 'exynos_adc_ops' structure which includes some functions
> to control ADC operation according to ADC version (v1 or v2).
>
> Signed-off-by: Chanwoo Choi 
> Acked-by: Kyungmin Park 

This is a good piece of change, Thanks.

Reviewed-by: Naveen Krishna Chatradhi 
> ---
>  drivers/iio/adc/exynos_adc.c | 174 
> +--
>  1 file changed, 120 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 010578f..c30def6 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -90,6 +90,7 @@ struct exynos_adc {
> struct clk  *clk;
> unsigned intirq;
> struct regulator*vdd;
> +   struct exynos_adc_ops   *ops;
>
> struct completion   completion;
>
> @@ -97,6 +98,13 @@ struct exynos_adc {
> unsigned intversion;
>  };
>
> +struct exynos_adc_ops {
> +   void (*init_hw)(struct exynos_adc *info);
> +   void (*clear_irq)(struct exynos_adc *info);
> +   void (*start_conv)(struct exynos_adc *info, unsigned long addr);
> +   void (*stop_conv)(struct exynos_adc *info);
> +};
> +
>  static const struct of_device_id exynos_adc_match[] = {
> { .compatible = "samsung,exynos-adc-v1", .data = (void *)ADC_V1 },
> { .compatible = "samsung,exynos-adc-v2", .data = (void *)ADC_V2 },
> @@ -112,30 +120,98 @@ static inline unsigned int 
> exynos_adc_get_version(struct platform_device *pdev)
> return (unsigned int)match->data;
>  }
>
> -static void exynos_adc_hw_init(struct exynos_adc *info)
> +static void exynos_adc_v1_init_hw(struct exynos_adc *info)
> +{
> +   u32 con1;
> +
> +   /* set default prescaler values and Enable prescaler */
> +   con1 =  ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
> +
> +   /* Enable 12-bit ADC resolution */
> +   con1 |= ADC_V1_CON_RES;
> +   writel(con1, ADC_V1_CON(info->regs));
> +}
> +
> +static void exynos_adc_v1_start_conv(struct exynos_adc *info, unsigned long 
> addr)
> +{
> +   u32 con1;
> +
> +   writel(addr, ADC_V1_MUX(info->regs));
> +
> +   con1 = readl(ADC_V1_CON(info->regs));
> +   writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
> +}
> +
> +static void exynos_adc_v1_clear_irq(struct exynos_adc *info)
> +{
> +   writel(1, ADC_V1_INTCLR(info->regs));
> +}
> +
> +static void exynos_adc_v1_stop_conv(struct exynos_adc *info)
> +{
> +   u32 con;
> +
> +   con = readl(ADC_V1_CON(info->regs));
> +   con |= ADC_V1_CON_STANDBY;
> +   writel(con, ADC_V1_CON(info->regs));
> +}
> +
> +static struct exynos_adc_ops exynos_adc_v1_ops = {
> +   .init_hw= exynos_adc_v1_init_hw,
> +   .clear_irq  = exynos_adc_v1_clear_irq,
> +   .start_conv = exynos_adc_v1_start_conv,
> +   .stop_conv  = exynos_adc_v1_stop_conv,
> +};
> +
> +static void exynos_adc_v2_init_hw(struct exynos_adc *info)
>  {
> u32 con1, con2;
>
> -   if (info->version == ADC_V2) {
> -   con1 = ADC_V2_CON1_SOFT_RESET;
> -   writel(con1, ADC_V2_CON1(info->regs));
> +   con1 = ADC_V2_CON1_SOFT_RESET;
> +   writel(con1, ADC_V2_CON1(info->regs));
>
> -   con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
> -   ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
> -   writel(con2, ADC_V2_CON2(info->regs));
> +   con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
> +   ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
> +   writel(con2, ADC_V2_CON2(info->regs));
>
> -   /* Enable interrupts */
> -   writel(1, ADC_V2_INT_EN(info->regs));
> -   } else {
> -   /* set default prescaler values and Enable prescaler */
> -   con1 =  ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
> +   /* Enable interrupts */
> +   writel(1, ADC_V2_INT_EN(info->regs));
> +}
>
> -   /* Enable 12-bit ADC resolution */
> -   con1 |= ADC_V1_CON_RES;
> -   writel(con1, ADC_V1_CON(info->regs));
> -   }
> +static void exynos_adc_v2_start_conv(struct exynos_adc *info, unsigned long 
> addr)
> +{
> +   u32 con1, con2;
> +
> +   con2 = readl(ADC_V2_CON2(info->regs));
> +   con2 &= ~ADC_V2_CON2_ACH_MASK;
> +   con2 |= ADC_V2_CON2_ACH_SEL(addr);
> +   writel(con2, ADC_V2_CON2(info->regs));
> +
> +   con1 = readl(ADC_V2_CON1(info->regs));
> +   writel(con1 | ADC_CON_EN_START, ADC_V2_CON1(info->regs));
> +}
> +
> +static void exynos_adc_v2_clear_irq(struct exynos_adc *info)
> +{
> +   writel(1, ADC_V2_INT_ST(info->regs));
> +}
> +
> +static void exynos_adc_v2_stop_conv(struct exynos_adc *info)
> +{
> +   u32 con;
> +
> +   con = readl(ADC_V2_CON1(info->regs));
> +   con &= ~ADC_CON_EN_START;
> +   writel(con, ADC_V2_CON1(info->regs));
>  }
>
> +sta

Re: [PATCH 10/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi Doug,

On 17 June 2014 21:54, Doug Anderson  wrote:
> Simon,
>
> On Tue, Jun 17, 2014 at 8:46 PM, Simon Glass  wrote:
>> Hi,
>>
>> On 16 June 2014 14:40, Doug Anderson  wrote:
>>> From: Bill Richardson 
>>>
>>> When communicating with the EC, the cmd_xfer() function should return the
>>> number of bytes it received from the EC, or negative on error.
>>
>> This is just for the I2C tunnel feature, right? If so, I think this
>> should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
>> cmd_xfer().
>
> No, the tunnel feature is implemented just fine without this (and is
> already landed and working).  It looks like the (not yet upstreamed)
> ec_i2c_limited_xfer for spring returns this new value directly but I'm
> not convinced that's technicall correct.
>
> Bill can correct me if I'm wrong, but I think this is primarily
> interesting once we add in cros_ec_dev (the userspace API) which needs
> this info.  Until that happens this patch doesn't hurt and just
> returns some extra info.

Agreed.

Reviewed-by: Simon Glass 

Regards,
Simon
--
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] drm/exynos: change zero to NULL for sparse

[Inki Dae]

On 2014년 06월 11일 15:36, Dan Carpenter wrote:
> 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

Applied.

Thanks,
Inki Dae

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

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

[Inki Dae]

On 2014년 06월 18일 09:09, Tomasz Figa wrote:
> On 10.06.2014 22:57, Tomasz Figa wrote:
>> 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;
>>
> 
> Ping.

Applied.

Thanks,
Inki Dae

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

--
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 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 17 June 2014 21:27, Doug Anderson  wrote:
> Simon,
>
> On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass  wrote:
>>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
>>> b/drivers/input/keyboard/cros_ec_keyb.c
>>> index 4083796..dc37b6b 100644
>>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>>> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>>>
>>>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
>>> *kb_state)
>>>  {
>>> -   return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>>> - kb_state, ckdev->cols);
>>> +   int ret;
>>> +   struct cros_ec_command msg = {
>>> +   .version = 0,
>>> +   .command = EC_CMD_MKBP_STATE,
>>> +   .outdata = NULL,
>>> +   .outsize = 0,
>>> +   .indata = kb_state,
>>> +   .insize = ckdev->cols,
>>> +   };
>>> +
>>> +   ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>>
>> Do we need ret?
>
> No.  If you wish I will spin without it.  Let me know.
>
> Note that locally this code includes a comment between the above and the 
> return:
>   /* FIXME: This assumes msg.result == EC_RES_SUCCESS */

It's not important to me, and you've explained the other question.

Reviewed-by: Simon Glass 
--
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 10/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:46 PM, Simon Glass  wrote:
> Hi,
>
> On 16 June 2014 14:40, Doug Anderson  wrote:
>> From: Bill Richardson 
>>
>> When communicating with the EC, the cmd_xfer() function should return the
>> number of bytes it received from the EC, or negative on error.
>
> This is just for the I2C tunnel feature, right? If so, I think this
> should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
> cmd_xfer().

No, the tunnel feature is implemented just fine without this (and is
already landed and working).  It looks like the (not yet upstreamed)
ec_i2c_limited_xfer for spring returns this new value directly but I'm
not convinced that's technicall correct.

Bill can correct me if I'm wrong, but I think this is primarily
interesting once we add in cros_ec_dev (the userspace API) which needs
this info.  Until that happens this patch doesn't hurt and just
returns some extra info.

-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 09/10] mfd: cros_ec: Check result code from EC messages

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:43 PM, Simon Glass  wrote:
>> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
>> index 09ca789..4d34f1c 100644
>> --- a/drivers/mfd/cros_ec_spi.c
>> +++ b/drivers/mfd/cros_ec_spi.c
>> @@ -289,21 +289,23 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
>> *ec_dev,
>> goto exit;
>> }
>>
>> -   /* check response error code */
>> ptr = ec_dev->din;
>> -   if (ptr[0]) {
>> -   if (ptr[0] == EC_RES_IN_PROGRESS) {
>> -   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
>> -   ec_msg->command);
>> -   ret = -EAGAIN;
>> -   goto exit;
>> -   }
>> -   dev_warn(ec_dev->dev, "command 0x%02x returned an error 
>> %d\n",
>> -ec_msg->command, ptr[0]);
>> -   debug_packet(ec_dev->dev, "in_err", ptr, len);
>> -   ret = -EINVAL;
>> +
>> +   /* check response error code */
>> +   ec_msg->result = ptr[0];
>> +   switch (ec_msg->result) {
>> +   case EC_RES_SUCCESS:
>> +   break;
>> +   case EC_RES_IN_PROGRESS:
>> +   ret = -EAGAIN;
>> +   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
>> +   ec_msg->command);
>> goto exit;
>> +   default:
>> +   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
>> +ec_msg->command, ec_msg->result);
>> }
>
> Since this code is the same as the above, should it go in a common
> function in cros_ec?

So you are thinking it should be written like:

ec_msg->result = ptr[0];
ret = cros_ec_check_result(ec_dev, ec_msg);
if (ret)
  goto exit;

---

int cros_ec_check_result(struct cros_ec_device *ec_dev, struct
cros_ec_command *ec_msg)
{
   switch (ec_msg->result) {
   case EC_RES_SUCCESS:
   return 0;
   case EC_RES_IN_PROGRESS:
   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
   ec_msg->command);
   return -EAGAIN;
   default:
   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
ec_msg->command, ec_msg->result);
   return 0;
}

OK, that seems reasonable.  I'll plan to spin tomorrow with that.

-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 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 9:25 PM, Simon Glass  wrote:
> Hi Doug,
>
> On 17 June 2014 21:22, Doug Anderson  wrote:
>>
>> Simon,
>>
>> On Tue, Jun 17, 2014 at 8:39 PM, Simon Glass  wrote:
>> > Hi Doug,
>> >
>> > On 16 June 2014 14:39, Doug Anderson  wrote:
>> >> From: Bill Richardson 
>> >>
>> >> struct cros_ec_device has a superfluous "name" field. We can get all the
>> >> debugging info we need from the existing ec_name and phys_name fields, so
>> >> let's take out the extra field.
>> >
>> > Except that it no longer prints I2C/SPI - i.e. the transport that is
>> > used. Is that not considered important?
>>
>> Before this change:
>>   [1.895472] cros-ec-spi spi2.0: Chrome EC (SPI)
>>
>> After this change:
>>   [1.910671] cros-ec-spi spi2.0: Chrome EC device registered
>>
>>
>> I think having SPI in the name twice is probably enough.  ;)
>
> Ah that helps! Could have been in the commit message.
>
> Reviewed-by: Simon Glass 

If I re-spin the series I'll add it.  I think the new message was in
the original commit in the "TEST=" clause and I left it out.  I
probably should have added it in with the proper wording...

-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 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:42 PM, Simon Glass  wrote:
>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
>> b/drivers/input/keyboard/cros_ec_keyb.c
>> index 4083796..dc37b6b 100644
>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>>
>>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
>> *kb_state)
>>  {
>> -   return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>> - kb_state, ckdev->cols);
>> +   int ret;
>> +   struct cros_ec_command msg = {
>> +   .version = 0,
>> +   .command = EC_CMD_MKBP_STATE,
>> +   .outdata = NULL,
>> +   .outsize = 0,
>> +   .indata = kb_state,
>> +   .insize = ckdev->cols,
>> +   };
>> +
>> +   ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);
>
> Do we need ret?

No.  If you wish I will spin without it.  Let me know.

Note that locally this code includes a comment between the above and the return:
  /* FIXME: This assumes msg.result == EC_RES_SUCCESS */

Given that this is not a new problem introduced in this code, that it
still hasn't been fixed locally in the ChromeOS tree, and that
generally FIXMEs don't seem to be left in the code upstream, I left it
out.

>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 2b0c598..60c0880 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -63,9 +63,10 @@ struct cros_ec_command {
>>   * @was_wake_device: true if this device was set to wake the system from
>>   * sleep at the last suspend
>>   * @event_notifier: interrupt event notifier for transport devices
>> - * @command_send: send a command
>> - * @command_recv: receive a response
>> - * @command_sendrecv: send a command and receive a response
>> + * @cmd_xfer: send command to EC and get response
>> + * Returns 0 if the communication succeeded, but that doesn't mean the 
>> EC
>> + * was happy with the command it got. Caller should check msg.result for
>> + * the EC's result code.
>>   *
>>   * @priv: Private data
>>   * @irq: Interrupt to use
>> @@ -83,7 +84,6 @@ struct cros_ec_command {
>>   * @parent: pointer to parent device (e.g. i2c or spi device)
>>   * @wake_enabled: true if this device can wake the system from sleep
>>   * @lock: one transaction at a time
>> - * @cmd_xfer: low-level channel to the EC
>>   */
>>  struct cros_ec_device {
>>
>> @@ -94,13 +94,8 @@ struct cros_ec_device {
>> bool was_wake_device;
>> struct class *cros_class;
>> struct blocking_notifier_head event_notifier;
>> -   int (*command_send)(struct cros_ec_device *ec,
>> -   uint16_t cmd, void *out_buf, int out_len);
>> -   int (*command_recv)(struct cros_ec_device *ec,
>> -   uint16_t cmd, void *in_buf, int in_len);
>> -   int (*command_sendrecv)(struct cros_ec_device *ec,
>> -   uint16_t cmd, void *out_buf, int out_len,
>> -   void *in_buf, int in_len);
>> +   int (*cmd_xfer)(struct cros_ec_device *ec,
>> +   struct cros_ec_command *msg);
>>
>> /* These are used to implement the platform-specific interface */
>> void *priv;
>> @@ -112,8 +107,6 @@ struct cros_ec_device {
>> struct device *parent;
>> bool wake_enabled;
>> struct mutex lock;
>> -   int (*cmd_xfer)(struct cros_ec_device *ec,
>> -   struct cros_ec_command *msg);
>
> Seems odd - maybe this line move of cmd_xfer() should be in an earlier patch?

It got moved from "private" to public.  Bill reorganized all the
public stuff at the top and the private at the bottom.

-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 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 17 June 2014 21:22, Doug Anderson  wrote:
>
> Simon,
>
> On Tue, Jun 17, 2014 at 8:39 PM, Simon Glass  wrote:
> > Hi Doug,
> >
> > On 16 June 2014 14:39, Doug Anderson  wrote:
> >> From: Bill Richardson 
> >>
> >> struct cros_ec_device has a superfluous "name" field. We can get all the
> >> debugging info we need from the existing ec_name and phys_name fields, so
> >> let's take out the extra field.
> >
> > Except that it no longer prints I2C/SPI - i.e. the transport that is
> > used. Is that not considered important?
>
> Before this change:
>   [1.895472] cros-ec-spi spi2.0: Chrome EC (SPI)
>
> After this change:
>   [1.910671] cros-ec-spi spi2.0: Chrome EC device registered
>
>
> I think having SPI in the name twice is probably enough.  ;)

Ah that helps! Could have been in the commit message.

Reviewed-by: Simon Glass 

Regards,
Simon
--
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 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:39 PM, Simon Glass  wrote:
> Hi Doug,
>
> On 16 June 2014 14:39, Doug Anderson  wrote:
>> From: Bill Richardson 
>>
>> struct cros_ec_device has a superfluous "name" field. We can get all the
>> debugging info we need from the existing ec_name and phys_name fields, so
>> let's take out the extra field.
>
> Except that it no longer prints I2C/SPI - i.e. the transport that is
> used. Is that not considered important?

Before this change:
  [1.895472] cros-ec-spi spi2.0: Chrome EC (SPI)

After this change:
  [1.910671] cros-ec-spi spi2.0: Chrome EC device registered


I think having SPI in the name twice is probably enough.  ;)

-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 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Doug Anderson]

Simon,

On Tue, Jun 17, 2014 at 8:35 PM, Simon Glass  wrote:
> Hi Doug,
>
> On 16 June 2014 14:39, Doug Anderson  wrote:
>> From: Bill Richardson 
>>
>> The members of struct cros_ec_device were improperly commented, and
>> intermixed the private and public sections. This is just cleanup to make it
>> more obvious what goes with what.
>>
>> [dianders: left lock in the structure but gave it the name that will
>> eventually be used.]
>>
>> Signed-off-by: Bill Richardson 
>> Signed-off-by: Doug Anderson 
>> ---
>>  drivers/mfd/cros_ec.c   |  2 +-
>>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>>  drivers/mfd/cros_ec_spi.c   | 10 +++
>>  include/linux/mfd/cros_ec.h | 65 
>> -
>>  4 files changed, 43 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index bd6f936..a9eede5 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
>> *ec_dev,
>> msg.in_buf = in_buf;
>> msg.in_len = in_len;
>>
>> -   return ec_dev->command_xfer(ec_dev, &msg);
>> +   return ec_dev->cmd_xfer(ec_dev, &msg);
>
> Why do this rename? It makes it different from the other members.

All I know of the history of this change is at
.  My best guess is that Bill was trying
to differentiate public vs. private function pointers.  Perhaps he
will chime in.

If it helps the other command_xxx() function pointers are removed in
the later "mfd: cros_ec: cleanup: Remove EC wrapper functions"

If you wish I can skip this rename, just let me know and it won't be
too much trouble.

-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 10/10] mfd: cros_ec: ec_dev->cmd_xfer() returns number of bytes received from EC

[Simon Glass]

Hi,

On 16 June 2014 14:40, Doug Anderson  wrote:
> From: Bill Richardson 
>
> When communicating with the EC, the cmd_xfer() function should return the
> number of bytes it received from the EC, or negative on error.

This is just for the I2C tunnel feature, right? If so, I think this
should be mentioned here. It seems to be affecting ec_i2c_xfer(), not
cmd_xfer().

>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
>  drivers/mfd/cros_ec_i2c.c   | 2 +-
>  drivers/mfd/cros_ec_spi.c   | 2 +-
>  include/linux/mfd/cros_ec.h | 8 
>  4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
> b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index dd07818..05e033c 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -228,7 +228,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> msg.insize = response_len;
>
> result = bus->ec->cmd_xfer(bus->ec, &msg);
> -   if (result)
> +   if (result < 0)
> goto exit;
>
> result = ec_i2c_parse_response(response, i2c_msgs, &num);
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 2276096..dc0ba29 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -120,7 +120,7 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
> *ec_dev,
> goto done;
> }
>
> -   ret = 0;
> +   ret = i2c_msg[1].buf[1];
>   done:
> kfree(in_buf);
> kfree(out_buf);
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 4d34f1c..beba1bc 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -333,7 +333,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
> *ec_dev,
> goto exit;
> }
>
> -   ret = 0;
> +   ret = len;
>  exit:
> mutex_unlock(&ec_spi->lock);
> return ret;
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 60c0880..7b65a75 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -41,7 +41,7 @@ enum {
>   * @outdata: Outgoing data to EC
>   * @outsize: Outgoing length in bytes
>   * @indata: Where to put the incoming data from EC
> - * @insize: Incoming length in bytes (filled in by EC)
> + * @insize: Max number of bytes to accept from EC
>   * @result: EC's response to the command (separate from communication 
> failure)
>   */
>  struct cros_ec_command {
> @@ -64,9 +64,9 @@ struct cros_ec_command {
>   * sleep at the last suspend
>   * @event_notifier: interrupt event notifier for transport devices
>   * @cmd_xfer: send command to EC and get response
> - * Returns 0 if the communication succeeded, but that doesn't mean the EC
> - * was happy with the command it got. Caller should check msg.result for
> - * the EC's result code.
> + * Returns the number of bytes received if the communication succeeded, 
> but
> + * that doesn't mean the EC was happy with the command. The caller
> + * should check msg.result for the EC's result code.
>   *
>   * @priv: Private data
>   * @irq: Interrupt to use
> --
> 2.0.0.526.g5318336
>

Regards,
Simon
--
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 09/10] mfd: cros_ec: Check result code from EC messages

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Just because the host was able to talk to the EC doesn't mean that the EC
> was happy with what it was told. Errors in communincation are not the same
> as error messages from the EC itself.
>
> This change lets the EC report its errors separately.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec_i2c.c | 15 +++
>  drivers/mfd/cros_ec_spi.c | 26 ++
>  2 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
> index 5bb32f5..2276096 100644
> --- a/drivers/mfd/cros_ec_i2c.c
> +++ b/drivers/mfd/cros_ec_i2c.c
> @@ -92,11 +92,18 @@ static int cros_ec_cmd_xfer_i2c(struct cros_ec_device 
> *ec_dev,
> }
>
> /* check response error code */
> -   if (i2c_msg[1].buf[0]) {
> -   dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -msg->command, i2c_msg[1].buf[0]);
> -   ret = -EINVAL;
> +   msg->result = i2c_msg[1].buf[0];
> +   switch (msg->result) {
> +   case EC_RES_SUCCESS:
> +   break;
> +   case EC_RES_IN_PROGRESS:
> +   ret = -EAGAIN;
> +   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> +   msg->command);
> goto done;
> +   default:
> +   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
> +msg->command, msg->result);
> }
>
> /* copy response packet payload and compute checksum */
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 09ca789..4d34f1c 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -289,21 +289,23 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device 
> *ec_dev,
> goto exit;
> }
>
> -   /* check response error code */
> ptr = ec_dev->din;
> -   if (ptr[0]) {
> -   if (ptr[0] == EC_RES_IN_PROGRESS) {
> -   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> -   ec_msg->command);
> -   ret = -EAGAIN;
> -   goto exit;
> -   }
> -   dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
> -ec_msg->command, ptr[0]);
> -   debug_packet(ec_dev->dev, "in_err", ptr, len);
> -   ret = -EINVAL;
> +
> +   /* check response error code */
> +   ec_msg->result = ptr[0];
> +   switch (ec_msg->result) {
> +   case EC_RES_SUCCESS:
> +   break;
> +   case EC_RES_IN_PROGRESS:
> +   ret = -EAGAIN;
> +   dev_dbg(ec_dev->dev, "command 0x%02x in progress\n",
> +   ec_msg->command);
> goto exit;
> +   default:
> +   dev_warn(ec_dev->dev, "command 0x%02x returned %d\n",
> +ec_msg->command, ec_msg->result);
> }

Since this code is the same as the above, should it go in a common
function in cros_ec?

> +
> len = ptr[1];
> sum = ptr[0] + ptr[1];
> if (len > ec_msg->insize) {
> --
> 2.0.0.526.g5318336
>

Regards,
Simon
--
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 08/10] mfd: cros_ec: cleanup: Remove EC wrapper functions

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Remove the three wrapper functions that talk to the EC without passing all
> the desired arguments and just use the underlying communication function
> that passes everything in a struct intead.
>
> This is internal code refactoring only. Nothing should change.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c | 15 +++
>  drivers/input/keyboard/cros_ec_keyb.c   | 14 --
>  drivers/mfd/cros_ec.c   | 32 
>  include/linux/mfd/cros_ec.h | 19 ++-
>  4 files changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c 
> b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 8e7a714..dd07818 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -183,6 +183,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> u8 *request = NULL;
> u8 *response = NULL;
> int result;
> +   struct cros_ec_command msg;
>
> request_len = ec_i2c_count_message(i2c_msgs, num);
> if (request_len < 0) {
> @@ -218,9 +219,15 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct 
> i2c_msg i2c_msgs[],
> }
>
> ec_i2c_construct_message(request, i2c_msgs, num, bus_num);
> -   result = bus->ec->command_sendrecv(bus->ec, EC_CMD_I2C_PASSTHRU,
> -  request, request_len,
> -  response, response_len);
> +
> +   msg.version = 0;
> +   msg.command = EC_CMD_I2C_PASSTHRU;
> +   msg.outdata = request;
> +   msg.outsize = request_len;
> +   msg.indata = response;
> +   msg.insize = response_len;
> +
> +   result = bus->ec->cmd_xfer(bus->ec, &msg);
> if (result)
> goto exit;
>
> @@ -258,7 +265,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
> u32 remote_bus;
> int err;
>
> -   if (!ec->command_sendrecv) {
> +   if (!ec->cmd_xfer) {
> dev_err(dev, "Missing sendrecv\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c 
> b/drivers/input/keyboard/cros_ec_keyb.c
> index 4083796..dc37b6b 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -191,8 +191,18 @@ static void cros_ec_keyb_close(struct input_dev *dev)
>
>  static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t 
> *kb_state)
>  {
> -   return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> - kb_state, ckdev->cols);
> +   int ret;
> +   struct cros_ec_command msg = {
> +   .version = 0,
> +   .command = EC_CMD_MKBP_STATE,
> +   .outdata = NULL,
> +   .outsize = 0,
> +   .indata = kb_state,
> +   .insize = ckdev->cols,
> +   };
> +
> +   ret = ckdev->ec->cmd_xfer(ckdev->ec, &msg);

Do we need ret?

> +   return ret;
>  }
>
>  static int cros_ec_keyb_work(struct notifier_block *nb,
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index d242714..6dd91e9 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -44,34 +44,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
>  }
>  EXPORT_SYMBOL(cros_ec_prepare_tx);
>
> -static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *out_buf, int out_len,
> -   void *in_buf, int in_len)
> -{
> -   struct cros_ec_command msg;
> -
> -   msg.version = cmd >> 8;
> -   msg.command = cmd & 0xff;
> -   msg.outdata = out_buf;
> -   msg.outsize = out_len;
> -   msg.indata = in_buf;
> -   msg.insize = in_len;
> -
> -   return ec_dev->cmd_xfer(ec_dev, &msg);
> -}
> -
> -static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *buf, int buf_len)
> -{
> -   return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
> -}
> -
> -static int cros_ec_command_send(struct cros_ec_device *ec_dev,
> -   uint16_t cmd, void *buf, int buf_len)
> -{
> -   return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
> -}
> -
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
> struct cros_ec_device *ec_dev = data;
> @@ -104,10 +76,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>
> BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
>
> -   ec_dev->command_send = cros_ec_command_send;
> -   ec_dev->command_recv = cros_ec_command_recv;
> -   ec_dev->command_sendrecv = cros_ec_command_sendrecv;
> -
> if (ec_dev->din_size) {
> ec_dev->din = devm_kzalloc(

Re: [PATCH 07/10] mfd: cros_ec: cleanup: remove unused fields from struct cros_ec_device

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> struct cros_ec_device has a superfluous "name" field. We can get all the
> debugging info we need from the existing ec_name and phys_name fields, so
> let's take out the extra field.

Except that it no longer prints I2C/SPI - i.e. the transport that is
used. Is that not considered important?

Anyway:

Reviewed-by: Simon Glass 

Regards,
Simon
--
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 04/10] mfd: cros_ec: Tweak struct cros_ec_device for clarity

[Simon Glass]

Hi Doug,

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> The members of struct cros_ec_device were improperly commented, and
> intermixed the private and public sections. This is just cleanup to make it
> more obvious what goes with what.
>
> [dianders: left lock in the structure but gave it the name that will
> eventually be used.]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 
> ---
>  drivers/mfd/cros_ec.c   |  2 +-
>  drivers/mfd/cros_ec_i2c.c   |  4 +--
>  drivers/mfd/cros_ec_spi.c   | 10 +++
>  include/linux/mfd/cros_ec.h | 65 
> -
>  4 files changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index bd6f936..a9eede5 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -57,7 +57,7 @@ static int cros_ec_command_sendrecv(struct cros_ec_device 
> *ec_dev,
> msg.in_buf = in_buf;
> msg.in_len = in_len;
>
> -   return ec_dev->command_xfer(ec_dev, &msg);
> +   return ec_dev->cmd_xfer(ec_dev, &msg);

Why do this rename? It makes it different from the other members.

Regards,
Simon
--
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 03/10] mfd: cros_ec: Allow static din/dout buffers with cros_ec_register()

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> The lower-level driver may want to provide its own buffers. If so,
> there's no need to allocate new ones.  This already happens to work
> just fine (since we check for size of 0 and use devm allocation), but
> it's good to document it.
>
> [dianders: Resolved conflicts; documented that no code changes needed
> on mainline]
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
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 02/10] mfd: cros_ec: IRQs for cros_ec should be optional

[Simon Glass]

On 16 June 2014 14:39, Doug Anderson  wrote:
> From: Bill Richardson 
>
> Preparing the way for the LPC device, which is just a plaform_device without
> interrupts.
>
> Signed-off-by: Bill Richardson 
> Signed-off-by: Doug Anderson 

Reviewed-by: Simon Glass 
--
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

[PATCHv4 0/4] iio: adc: exynos_adc: Support Exynos3250 ADC and code clean

[Chanwoo Choi]

This patchset support Exynos3250 ADC (Analog Digital Converter) because
Exynos3250 has additional special clock for ADC IP and add 'exynos_adc_ops'
structure to improve readability.

Changes from v3:
- Add new 'exynos_adc_ops' structure to improve readability according to
 Tomasz Figa comment[1]
 [1] https://lkml.org/lkml/2014/4/16/238
- Add new 'exynos3250-adc-v2' compatible string to support Exynos3250 ADC
- Fix wrong compaitlbe string of ADC in Exynos3250 dtsi file

Changes from v2:
- Check return value of clock function to deal with error exception
- Fix minor coding style to improve readability

Changes from v1:
- Add new "samsung,exynos-adc-v3" compatible to support Exynos3250 ADC
- Add a patch about DT binding documentation

Chanwoo Choi (4):
  iio: adc: exynos_adc: Add exynos_adc_ops structure to improve readability
  iio: adc: exynos_adc: Control special clock of ADC to support Exynos3250 ADC
  iio: devicetree: Add DT binding documentation for Exynos3250 ADC
  ARM: dts: Fix wrong compatible string for Exynos3250 ADC

 .../devicetree/bindings/arm/samsung/exynos-adc.txt |  20 ++
 arch/arm/boot/dts/exynos3250.dtsi  |   4 +-
 drivers/iio/adc/exynos_adc.c   | 267 -
 3 files changed, 223 insertions(+), 68 deletions(-)

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

[PATCHv4 3/4] iio: devicetree: Add DT binding documentation for Exynos3250 ADC

[Chanwoo Choi]

This patch add DT binding documentation for Exynos3250 ADC IP. Exynos3250 has
special clock ('sclk_tsadc') for ADC which provide clock to internal ADC.

Signed-off-by: Chanwoo Choi 
Acked-by: Kyungmin Park 
---
 .../devicetree/bindings/arm/samsung/exynos-adc.txt   | 20 
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt 
b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index 5d49f2b..3a5af82 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -14,6 +14,8 @@ Required properties:
for exynos4412/5250 controllers.
Must be "samsung,exynos-adc-v2" for
future controllers.
+   Must be "samsung,exynos3250-adc-v2" for
+   for exynos3250 controllers.
 - reg: Contains ADC register address range (base address and
length) and the address of the phy enable register.
 - interrupts:  Contains the interrupt information for the timer. The
@@ -21,7 +23,11 @@ Required properties:
the Samsung device uses.
 - #io-channel-cells = <1>; As ADC has multiple outputs
 - clocks   From common clock binding: handle to adc clock.
+   From common clock binding: handle to sclk_tsadc clock
+   if using Exynos3250.
 - clock-names  From common clock binding: Shall be "adc".
+   From common clock binding: Shall be "sclk_tsadc"
+   if using Exynos3250.
 - vdd-supply   VDD input supply.
 
 Note: child nodes can be added for auto probing from device tree.
@@ -41,6 +47,20 @@ adc: adc@12D1 {
vdd-supply = <&buck5_reg>;
 };
 
+Example: adding device info in dtsi file for Exynos3250 with additional sclk
+
+adc: adc@126C {
+   compatible = "samsung,exynos3250-adc-v2";
+   reg = <0x126C 0x100>, <0x10020718 0x4>;
+   interrupts = <0 137 0>;
+   #io-channel-cells = <1>;
+   io-channel-ranges;
+
+   clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
+   clock-names = "adc", "sclk_adc";
+
+   vdd-supply = <&buck5_reg>;
+};
 
 Example: Adding child nodes in dts file
 
-- 
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

[PATCHv4 4/4] ARM: dts: Fix wrong compatible string for Exynos3250 ADC

[Chanwoo Choi]

This patchset fix wrong compatible string for Exynos3250 ADC. Exynos3250 SoC
need to control only special clock for ADC. Exynos SoC except for Exynos3250
has not included special clock for ADC. The exynos ADC driver can control
special clock if compatible string is 'exynos3250-adc-v2'.

Signed-off-by: Chanwoo Choi 
Acked-by: Kyungmin Park 
---
 arch/arm/boot/dts/exynos3250.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi 
b/arch/arm/boot/dts/exynos3250.dtsi
index 6c1fb67..107dc44 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -414,10 +414,10 @@
};
 
adc: adc@126C {
-   compatible = "samsung,exynos-adc-v3";
+   compatible = "samsung,exynos3250-adc-v2";
reg = <0x126C 0x100>, <0x10020718 0x4>;
interrupts = <0 137 0>;
-   clock-names = "adc", "sclk_tsadc";
+   clock-names = "adc", "sclk_adc";
clocks = <&cmu CLK_TSADC>, <&cmu CLK_SCLK_TSADC>;
#io-channel-cells = <1>;
io-channel-ranges;
-- 
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

[PATCHv4 2/4] iio: adc: exynos_adc: Control special clock of ADC to support Exynos3250 ADC

[Chanwoo Choi]

This patch control special clock for ADC in Exynos series's FSYS block.
If special clock of ADC is registerd on clock list of common clk framework,
Exynos ADC drvier have to control this clock.

Exynos3250/Exynos4/Exynos5 has 'adc' clock as following:
- 'adc' clock: bus clock for ADC

Exynos3250 has additional 'sclk_adc' clock as following:
- 'sclk_adc' clock: special clock for ADC which provide clock to internal ADC

Exynos 4210/4212/4412 and Exynos5250/5420 has not included 'sclk_adc' clock
in FSYS_BLK. But, Exynos3250 based on Cortex-A7 has only included 'sclk_adc'
clock in FSYS_BLK.

Signed-off-by: Chanwoo Choi 
Acked-by: Kyungmin Park 
---
 drivers/iio/adc/exynos_adc.c | 93 ++--
 1 file changed, 81 insertions(+), 12 deletions(-)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index c30def6..6b026ac 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -41,7 +41,8 @@
 
 enum adc_version {
ADC_V1,
-   ADC_V2
+   ADC_V2,
+   ADC_V2_EXYNOS3250,
 };
 
 /* EXYNOS4412/5250 ADC_V1 registers definitions */
@@ -85,9 +86,11 @@ enum adc_version {
 #define EXYNOS_ADC_TIMEOUT (msecs_to_jiffies(100))
 
 struct exynos_adc {
+   struct device   *dev;
void __iomem*regs;
void __iomem*enable_reg;
struct clk  *clk;
+   struct clk  *sclk;
unsigned intirq;
struct regulator*vdd;
struct exynos_adc_ops   *ops;
@@ -96,6 +99,7 @@ struct exynos_adc {
 
u32 value;
unsigned intversion;
+   boolneeds_sclk;
 };
 
 struct exynos_adc_ops {
@@ -103,11 +107,21 @@ struct exynos_adc_ops {
void (*clear_irq)(struct exynos_adc *info);
void (*start_conv)(struct exynos_adc *info, unsigned long addr);
void (*stop_conv)(struct exynos_adc *info);
+   void (*disable_clk)(struct exynos_adc *info);
+   int (*enable_clk)(struct exynos_adc *info);
 };
 
 static const struct of_device_id exynos_adc_match[] = {
-   { .compatible = "samsung,exynos-adc-v1", .data = (void *)ADC_V1 },
-   { .compatible = "samsung,exynos-adc-v2", .data = (void *)ADC_V2 },
+   {
+   .compatible = "samsung,exynos-adc-v1",
+   .data = (void *)ADC_V1,
+   }, {
+   .compatible = "samsung,exynos-adc-v2",
+   .data = (void *)ADC_V2,
+   }, {
+   .compatible = "samsung,exynos3250-adc-v2",
+   .data = (void *)ADC_V2_EXYNOS3250,
+   },
{},
 };
 MODULE_DEVICE_TABLE(of, exynos_adc_match);
@@ -156,11 +170,42 @@ static void exynos_adc_v1_stop_conv(struct exynos_adc 
*info)
writel(con, ADC_V1_CON(info->regs));
 }
 
+static void exynos_adc_disable_clk(struct exynos_adc *info)
+{
+   if (info->needs_sclk)
+   clk_disable_unprepare(info->sclk);
+   clk_disable_unprepare(info->clk);
+}
+
+static int exynos_adc_enable_clk(struct exynos_adc *info)
+{
+   int ret;
+
+   ret = clk_prepare_enable(info->clk);
+   if (ret) {
+   dev_err(info->dev, "failed enabling adc clock: %d\n", ret);
+   return ret;
+   }
+
+   if (info->needs_sclk) {
+   ret = clk_prepare_enable(info->sclk);
+   if (ret) {
+   clk_disable_unprepare(info->clk);
+   dev_err(info->dev,
+   "failed enabling sclk_tsadc clock: %d\n", ret);
+   }
+   }
+
+   return 0;
+}
+
 static struct exynos_adc_ops exynos_adc_v1_ops = {
.init_hw= exynos_adc_v1_init_hw,
.clear_irq  = exynos_adc_v1_clear_irq,
.start_conv = exynos_adc_v1_start_conv,
.stop_conv  = exynos_adc_v1_stop_conv,
+   .disable_clk= exynos_adc_disable_clk,
+   .enable_clk = exynos_adc_enable_clk,
 };
 
 static void exynos_adc_v2_init_hw(struct exynos_adc *info)
@@ -210,6 +255,8 @@ static struct exynos_adc_ops exynos_adc_v2_ops = {
.start_conv = exynos_adc_v2_start_conv,
.clear_irq  = exynos_adc_v2_clear_irq,
.stop_conv  = exynos_adc_v2_stop_conv,
+   .disable_clk= exynos_adc_disable_clk,
+   .enable_clk = exynos_adc_enable_clk,
 };
 
 static int exynos_read_raw(struct iio_dev *indio_dev,
@@ -345,6 +392,10 @@ static int exynos_adc_probe(struct platform_device *pdev)
case ADC_V2:
info->ops = &exynos_adc_v2_ops;
break;
+   case ADC_V2_EXYNOS3250:
+   info->ops = &exynos_adc_v2_ops;
+   info->needs_sclk = true;
+   break;
default:
dev_err(&pdev->dev, "failed to identify ADC version\n");
return -EINVAL;
@@ -367,6 +418,7 @@ static int exynos_adc_probe(struct platform_device *pdev)
}
 
info->irq = irq;
+

[PATCHv4 1/4] iio: adc: exynos_adc: Add exynos_adc_ops structure to improve readability

[Chanwoo Choi]

This patchset add 'exynos_adc_ops' structure which includes some functions
to control ADC operation according to ADC version (v1 or v2).

Signed-off-by: Chanwoo Choi 
Acked-by: Kyungmin Park 
---
 drivers/iio/adc/exynos_adc.c | 174 +--
 1 file changed, 120 insertions(+), 54 deletions(-)

diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 010578f..c30def6 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -90,6 +90,7 @@ struct exynos_adc {
struct clk  *clk;
unsigned intirq;
struct regulator*vdd;
+   struct exynos_adc_ops   *ops;
 
struct completion   completion;
 
@@ -97,6 +98,13 @@ struct exynos_adc {
unsigned intversion;
 };
 
+struct exynos_adc_ops {
+   void (*init_hw)(struct exynos_adc *info);
+   void (*clear_irq)(struct exynos_adc *info);
+   void (*start_conv)(struct exynos_adc *info, unsigned long addr);
+   void (*stop_conv)(struct exynos_adc *info);
+};
+
 static const struct of_device_id exynos_adc_match[] = {
{ .compatible = "samsung,exynos-adc-v1", .data = (void *)ADC_V1 },
{ .compatible = "samsung,exynos-adc-v2", .data = (void *)ADC_V2 },
@@ -112,30 +120,98 @@ static inline unsigned int exynos_adc_get_version(struct 
platform_device *pdev)
return (unsigned int)match->data;
 }
 
-static void exynos_adc_hw_init(struct exynos_adc *info)
+static void exynos_adc_v1_init_hw(struct exynos_adc *info)
+{
+   u32 con1;
+
+   /* set default prescaler values and Enable prescaler */
+   con1 =  ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
+
+   /* Enable 12-bit ADC resolution */
+   con1 |= ADC_V1_CON_RES;
+   writel(con1, ADC_V1_CON(info->regs));
+}
+
+static void exynos_adc_v1_start_conv(struct exynos_adc *info, unsigned long 
addr)
+{
+   u32 con1;
+
+   writel(addr, ADC_V1_MUX(info->regs));
+
+   con1 = readl(ADC_V1_CON(info->regs));
+   writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
+}
+
+static void exynos_adc_v1_clear_irq(struct exynos_adc *info)
+{
+   writel(1, ADC_V1_INTCLR(info->regs));
+}
+
+static void exynos_adc_v1_stop_conv(struct exynos_adc *info)
+{
+   u32 con;
+
+   con = readl(ADC_V1_CON(info->regs));
+   con |= ADC_V1_CON_STANDBY;
+   writel(con, ADC_V1_CON(info->regs));
+}
+
+static struct exynos_adc_ops exynos_adc_v1_ops = {
+   .init_hw= exynos_adc_v1_init_hw,
+   .clear_irq  = exynos_adc_v1_clear_irq,
+   .start_conv = exynos_adc_v1_start_conv,
+   .stop_conv  = exynos_adc_v1_stop_conv,
+};
+
+static void exynos_adc_v2_init_hw(struct exynos_adc *info)
 {
u32 con1, con2;
 
-   if (info->version == ADC_V2) {
-   con1 = ADC_V2_CON1_SOFT_RESET;
-   writel(con1, ADC_V2_CON1(info->regs));
+   con1 = ADC_V2_CON1_SOFT_RESET;
+   writel(con1, ADC_V2_CON1(info->regs));
 
-   con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
-   ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
-   writel(con2, ADC_V2_CON2(info->regs));
+   con2 = ADC_V2_CON2_OSEL | ADC_V2_CON2_ESEL |
+   ADC_V2_CON2_HIGHF | ADC_V2_CON2_C_TIME(0);
+   writel(con2, ADC_V2_CON2(info->regs));
 
-   /* Enable interrupts */
-   writel(1, ADC_V2_INT_EN(info->regs));
-   } else {
-   /* set default prescaler values and Enable prescaler */
-   con1 =  ADC_V1_CON_PRSCLV(49) | ADC_V1_CON_PRSCEN;
+   /* Enable interrupts */
+   writel(1, ADC_V2_INT_EN(info->regs));
+}
 
-   /* Enable 12-bit ADC resolution */
-   con1 |= ADC_V1_CON_RES;
-   writel(con1, ADC_V1_CON(info->regs));
-   }
+static void exynos_adc_v2_start_conv(struct exynos_adc *info, unsigned long 
addr)
+{
+   u32 con1, con2;
+
+   con2 = readl(ADC_V2_CON2(info->regs));
+   con2 &= ~ADC_V2_CON2_ACH_MASK;
+   con2 |= ADC_V2_CON2_ACH_SEL(addr);
+   writel(con2, ADC_V2_CON2(info->regs));
+
+   con1 = readl(ADC_V2_CON1(info->regs));
+   writel(con1 | ADC_CON_EN_START, ADC_V2_CON1(info->regs));
+}
+
+static void exynos_adc_v2_clear_irq(struct exynos_adc *info)
+{
+   writel(1, ADC_V2_INT_ST(info->regs));
+}
+
+static void exynos_adc_v2_stop_conv(struct exynos_adc *info)
+{
+   u32 con;
+
+   con = readl(ADC_V2_CON1(info->regs));
+   con &= ~ADC_CON_EN_START;
+   writel(con, ADC_V2_CON1(info->regs));
 }
 
+static struct exynos_adc_ops exynos_adc_v2_ops = {
+   .init_hw= exynos_adc_v2_init_hw,
+   .start_conv = exynos_adc_v2_start_conv,
+   .clear_irq  = exynos_adc_v2_clear_irq,
+   .stop_conv  = exynos_adc_v2_stop_conv,
+};
+
 static int exynos_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
in

Re: [PATCH] drm/exynos: dpi: Fix NULL pointer dereference with legacy bindings

[Jingoo Han]

On Wednesday, June 11, 2014 5:58 AM, Tomasz Figa wrote:
> 
> 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 

(+cc Inki Dae)

Reviewed-by: Jingoo Han 

Best regards,
Jingoo Han

> ---
>  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;
> --

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

[Tomasz Figa]

On 10.06.2014 22:57, Tomasz Figa wrote:
> 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;
> 

Ping.

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] devicetree: Add generic IOMMU device tree bindings

[Thierry Reding]

On Tue, Jun 17, 2014 at 01:18:11PM +0100, Will Deacon wrote:
> On Tue, Jun 17, 2014 at 12:58:30PM +0100, Thierry Reding wrote:
> > On Mon, Jun 16, 2014 at 01:57:04PM +0100, Will Deacon wrote:
> > > On Wed, Jun 04, 2014 at 10:12:38PM +0100, Thierry Reding wrote:
> > > > It can easily be argued that if the algorithm used to remap the ID
> > > > varies, the compatibility of the device changes. Therefore I would
> > > > expect any variant of the GICv3 that deviates from the "standard"
> > > > mapping (if there is such a thing) to have its own compatible string.
> > > 
> > > There is no standard mapping; it's a property defined at system 
> > > integration
> > > time. I fully expect different SoCs to do different things here.
> > 
> > My point was that the mapping itself seems to be fundamental enough to
> > make devices with different mappings "incompatible". Therefore I think
> > this could probably be handled by using different compatible values,
> > something along the lines of this:
> > 
> > compatible = "vendor,soc-gicv3", "arm,gicv3";
> > 
> > Then the mapping can be described in code, which should be a whole lot
> > easier and more flexible than a more or less generic notation in device
> > tree.
> 
> I don't think that scales well beyond a handful of unique mappings, and I
> really anticipate everybody doing something different based on their
> integration constraints.
> 
> You'd very quickly end up with sets of tables for each SoC, describing the
> topology and associated IDs in the kernel source, which feels like a giant
> step backwards from where we are today with device tree.

Well, today we don't have a generic binding at all, so anything will
really be a giant step forward in my opinion.

But seriously, from what you said earlier I got the impression that some
of the mappings may not be easy or possible to represent in DT, which is
why I proposed to encode it into the compatible property so that it can
be handled in code instead.

We've had similar discussions before (power sequences anyone?) where we
tried to come up with a generic way to describe something in device tree
that just didn't work out too well. Some things are better done in code,
so I think we should at least consider that possibility rather than
blindly try and force everything into device tree.

Thierry


pgpINMokP3WHH.pgp
Description: PGP signature

Re: [PATCH v2 01/10] mfd: max77686: Convert to use regmap_irq

[Doug Anderson]

Javier,

On Mon, Jun 16, 2014 at 11:02 AM, Javier Martinez Canillas
 wrote:
> @@ -127,15 +175,48 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
> }
> i2c_set_clientdata(max77686->rtc, max77686);
>
> -   max77686_irq_init(max77686);
> +   max77686->rtc_regmap = devm_regmap_init_i2c(max77686->rtc,
> +   
> &max77686_rtc_regmap_config);
> +   if (IS_ERR(max77686->rtc_regmap)) {
> +   ret = PTR_ERR(max77686->rtc_regmap);
> +   dev_err(max77686->dev, "failed to allocate RTC regmap: %d\n",
> +   ret);
> +   goto err_unregister_i2c;
> +   }
> +
> +   ret = regmap_add_irq_chip(max77686->regmap, max77686->irq,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
> + IRQF_SHARED, 0, &max77686_irq_chip,
> + &max77686->irq_data);
> +   if (ret != 0) {
> +   dev_err(&i2c->dev, "failed to add PMIC irq chip: %d\n", ret);
> +   goto err_unregister_i2c;
> +   }
> +   ret = regmap_add_irq_chip(max77686->rtc_regmap, max77686->irq,
> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
> + IRQF_SHARED, 0, &max77686_rtc_irq_chip,
> + &max77686->rtc_irq_data);
> +   if (ret != 0) {
> +   dev_err(&i2c->dev, "failed to add RTC irq chip: %d\n", ret);
> +   goto err_del_irqc;
> +   }
>
> ret = mfd_add_devices(max77686->dev, -1, max77686_devs,
>   ARRAY_SIZE(max77686_devs), NULL, 0, NULL);
> if (ret < 0) {
> -   mfd_remove_devices(max77686->dev);
> -   i2c_unregister_device(max77686->rtc);
> +   dev_err(&i2c->dev, "failed to add MFD devices: %d\n", ret);
> +   goto err_del_rtc_irqc;
> }
>
> +   return 0;
> +
> +err_del_rtc_irqc:
> +   regmap_del_irq_chip(max77686->irq, max77686->rtc_irq_data);
> +err_del_irqc:
> +   regmap_del_irq_chip(max77686->irq, max77686->irq_data);

I would imagine you either don't need these regmap_del_irq_chip() here
or that you _do_ need them in max77686_i2c_remove().

...from looking at other drivers I think the answer is to add them to
max77686_i2c_remove().


> diff --git a/include/linux/mfd/max77686-private.h 
> b/include/linux/mfd/max77686-private.h
> index 8c75a9c..3a810b1 100644
> --- a/include/linux/mfd/max77686-private.h
> +++ b/include/linux/mfd/max77686-private.h
> @@ -205,7 +205,7 @@ enum max77686_irq {
> MAX77686_PMICIRQ_140C,
> MAX77686_PMICIRQ_120C,
>
> -   MAX77686_RTCIRQ_RTC60S,
> +   MAX77686_RTCIRQ_RTC60S = 0,
> MAX77686_RTCIRQ_RTCA1,
> MAX77686_RTCIRQ_RTCA2,
> MAX77686_RTCIRQ_SMPL,
> @@ -215,6 +215,25 @@ enum max77686_irq {
> MAX77686_IRQ_NR,

Maybe remove MAX77686_IRQ_NR which no longer makes any sense now that
you start over at 0 partway through.


Overall this looks good to me, so once nits above are fixed feel to
add my Reviewed-by.  I've also built and booted this patch on
exynos5250-snow and tested that the RTC wakealarm fires and can even
wake the system up (with some additional work that I'll email you
about).

Reviewed-by: Doug Anderson 
Tested-by: Doug Anderson 
--
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 RFC] mfd: syscon: Decouple syscon interface from syscon devices

[Tomasz Figa]

Hi Arnd,

On 17.06.2014 17:42, Arnd Bergmann wrote:
> On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
>> Currently a syscon entity can be only registered directly through a
>> platform device that binds to a dedicated driver. However in certain use
>> cases it is desirable to make a device used with another driver a syscon
>> interface provider. For example, certain SoCs (e.g. Exynos) contain
>> system controller blocks which perform various functions such as power
>> domain control, CPU power management, low power mode control, but in
>> addition contain certain IP integration glue, such as various signal
>> masks, coprocessor power control, etc. In such case, there is a need to
>> have a dedicated driver for such system controller but also share
>> registers with other drivers. The latter is where the syscon interface
>> is helpful.
>>
>> This patch decouples syscon object from syscon driver, so that it can be
>> registered from any driver in addition to the original "syscon" platform
>> driver.
>>
>> Signed-off-by: Tomasz Figa 
> 
> Hi Tomasz,
> 
> This seems like a reasonable way of solving the problem, but I think
> there is an even better one that we have about in the past: if we
> promote syscon from a platform driver into a a drivers/base/ helper
> that is independent of the platform device matching, we can use
> call syscon_regmap_lookup_* for any device node, whether it's already
> bound to a driver or not, which do what you need. It would also make
> it easier to call the syscon code before the platform_device
> infrastructure gets initialized, which is something a number of
> people have asked for, e.g. for using regmap to do SMP bringup
> or for clock registration.

Basically, unless I'm missing your point, this is what my patch does,
except that I don't move it to drivers/base/ and the registration
function I added require a pointer to struct device. Indeed, decoupling
it further from the driver model, by adding of_syscon_register() should
be useful for early users.

Should I move this to drivers/base/, even though from current location
it can be used outside the platform driver anyway?

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 07/10] regulator: Add driver for Maxim 77802 PMIC regulators

[Lee Jones]

> The MAX77802 PMIC has 10 high-efficiency Buck and 32 Low-dropout
> (LDO) regulators. This patch adds support for all these regulators
> found on the MAX77802 PMIC and is based on a driver added by Simon
> Glass to the Chrome OS kernel 3.8 tree.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
> Changes since v1:
>  - Remove unneeded check if num_regulators != MAX77802_MAX_REGULATORS.
>  - Fix .set_suspend_mode handler comment and split regulators ops for
>regulators that behave differently. Suggested by Mark Brown.
>  - Use module_platform_driver() instead of having init/exit functions.
>Suggested by Mark Brown.
>  - Use the new descriptor-based GPIO interface instead of the deprecated
>integer based GPIO one. Suggested by Mark Brown.
>  - Look for "regulators" child node instead of "voltage-regulators" to be
>consistent with other PMIC drivers. Suggested by Mark Brown.
> 
>  drivers/mfd/max77802.c   |   1 +

Can you remove all of the MFD changes from patches 7, 8 and 9 and
create new one.  That way there's no requirement for any cross
subsystem messiness.

>  drivers/regulator/Kconfig|   9 +
>  drivers/regulator/Makefile   |   1 +
>  drivers/regulator/max77802.c | 701 
> +++
>  4 files changed, 712 insertions(+)
>  create mode 100644 drivers/regulator/max77802.c

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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 01/10] mfd: max77686: Convert to use regmap_irq

[Lee Jones]

On Mon, 16 Jun 2014, Javier Martinez Canillas wrote:

> By using the generic IRQ support in the Register map API, it
> is possible to get rid of max77686-irq.c and simplify the code.
> 
> Suggested-by: Krzysztof Kozlowski 
> Signed-off-by: Javier Martinez Canillas 
> ---
>  drivers/mfd/Kconfig  |   1 +
>  drivers/mfd/Makefile |   2 +-
>  drivers/mfd/max77686-irq.c   | 319 
> ---
>  drivers/mfd/max77686.c   |  93 +-
>  drivers/rtc/rtc-max77686.c   |  27 +--
>  include/linux/mfd/max77686-private.h |  26 ++-
>  include/linux/mfd/max77686.h |   2 -
>  7 files changed, 119 insertions(+), 351 deletions(-)
>  delete mode 100644 drivers/mfd/max77686-irq.c

Nice patch - great diff.

I assume we have to wait for some of the other patches in the set, but
for now:

  Acked-by: Lee Jones 

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index ee8204c..0916447 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -371,6 +371,7 @@ config MFD_MAX77686
>   depends on I2C=y
>   select MFD_CORE
>   select REGMAP_I2C
> + select REGMAP_IRQ
>   select IRQ_DOMAIN
>   help
> Say yes here to add support for Maxim Semiconductor MAX77686.
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 8afedba..3b3b408 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -115,7 +115,7 @@ da9063-objs   := da9063-core.o 
> da9063-irq.o da9063-i2c.o
>  obj-$(CONFIG_MFD_DA9063) += da9063.o
>  
>  obj-$(CONFIG_MFD_MAX14577)   += max14577.o
> -obj-$(CONFIG_MFD_MAX77686)   += max77686.o max77686-irq.o
> +obj-$(CONFIG_MFD_MAX77686)   += max77686.o
>  obj-$(CONFIG_MFD_MAX77693)   += max77693.o max77693-irq.o
>  obj-$(CONFIG_MFD_MAX8907)+= max8907.o
>  max8925-objs := max8925-core.o max8925-i2c.o
> diff --git a/drivers/mfd/max77686-irq.c b/drivers/mfd/max77686-irq.c
> deleted file mode 100644
> index cdc3280..000
> --- a/drivers/mfd/max77686-irq.c
> +++ /dev/null
> @@ -1,319 +0,0 @@
> -/*
> - * max77686-irq.c - Interrupt controller support for MAX77686
> - *
> - * Copyright (C) 2012 Samsung Electronics Co.Ltd
> - * Chiwoong Byun 
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> - *
> - * This driver is based on max8997-irq.c
> - */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -enum {
> - MAX77686_DEBUG_IRQ_INFO = 1 << 0,
> - MAX77686_DEBUG_IRQ_MASK = 1 << 1,
> - MAX77686_DEBUG_IRQ_INT = 1 << 2,
> -};
> -
> -static int debug_mask = 0;
> -module_param(debug_mask, int, 0);
> -MODULE_PARM_DESC(debug_mask, "Set debug_mask : 0x0=off 0x1=IRQ_INFO  
> 0x2=IRQ_MASK 0x4=IRQ_INI)");
> -
> -static const u8 max77686_mask_reg[] = {
> - [PMIC_INT1] = MAX77686_REG_INT1MSK,
> - [PMIC_INT2] = MAX77686_REG_INT2MSK,
> - [RTC_INT] = MAX77686_RTC_INTM,
> -};
> -
> -static struct regmap *max77686_get_regmap(struct max77686_dev *max77686,
> - enum max77686_irq_source src)
> -{
> - switch (src) {
> - case PMIC_INT1 ... PMIC_INT2:
> - return max77686->regmap;
> - case RTC_INT:
> - return max77686->rtc_regmap;
> - default:
> - return ERR_PTR(-EINVAL);
> - }
> -}
> -
> -struct max77686_irq_data {
> - int mask;
> - enum max77686_irq_source group;
> -};
> -
> -#define DECLARE_IRQ(idx, _group, _mask)  \
> - [(idx)] = { .group = (_group), .mask = (_mask) }
> -static const struct max77686_irq_data max77686_irqs[] = {
> - DECLARE_IRQ(MAX77686_PMICIRQ_PWRONF,PMIC_INT1, 1 << 0),
> - DECLARE_IRQ(MAX77686_PMICIRQ_PWRONR,PMIC_INT1, 1 << 1),
> - DECLARE_IRQ(MAX77686_PMICIRQ_JIGONBF,   PMIC_INT1, 1 << 2),
> - DECLARE_IRQ(MAX77686_PMICIRQ_JIGONBR,   PMIC_INT1, 1 << 3),
> - DECLARE_IRQ(MAX77686_PMICIRQ_ACOKBF,PMIC_INT1, 1 << 4),
> - DECLARE_IRQ(MAX77686_PMICIRQ_ACOKBR,PMIC_INT1, 1 << 5),
> - DECLARE_IRQ(MAX77686_PMICIRQ_ONKEY1S,   PMIC_INT1, 1 << 6),
> - DECLARE_IRQ(MAX77686_PMICIRQ_MRSTB, PMIC_INT1, 1 << 7),
> - DECLARE_IRQ(MAX77686_PMICIRQ_140C,  PMIC_INT2, 1 << 0),
> - DECLARE_IRQ(

[PATCH v2 6/9] thermal: exynos: simplify temp_to_code() and code_to_temp()

[Bartlomiej Zolnierkiewicz]

* Remove dead temp check from temp_to_code() (this function users
  in exynos_tmu_initialize() always pass correct temperatures and
  exynos_tmu_set_emulation() returns early for EXYNOS4210 because
  TMU_SUPPORT_EMULATION flag is not set on this SoC).

* Move temp_code check from code_to_temp() to exynos_tmu_read()
  (code_to_temp() only user).

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
Reviewed-by: Amit Daniel Kachhap
---
 drivers/thermal/samsung/exynos_tmu.c | 38 +++-
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 204f811..34ac081 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -72,19 +72,7 @@ struct exynos_tmu_data {
  */
 static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 {
-   struct exynos_tmu_platform_data *pdata = data->pdata;
-   int temp_code;
-
-   if (data->soc == SOC_ARCH_EXYNOS4210)
-   /* temp should range between 25 and 125 */
-   if (temp < 25 || temp > 125) {
-   temp_code = -EINVAL;
-   goto out;
-   }
-
-   temp_code = temp + data->temp_error - pdata->first_point_trim;
-out:
-   return temp_code;
+   return temp + data->temp_error - data->pdata->first_point_trim;
 }
 
 /*
@@ -93,19 +81,7 @@ out:
  */
 static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code)
 {
-   struct exynos_tmu_platform_data *pdata = data->pdata;
-   int temp;
-
-   if (data->soc == SOC_ARCH_EXYNOS4210)
-   /* temp_code should range between 75 and 175 */
-   if (temp_code < 75 || temp_code > 175) {
-   temp = -ENODATA;
-   goto out;
-   }
-
-   temp = temp_code - data->temp_error + pdata->first_point_trim;
-out:
-   return temp;
+   return temp_code - data->temp_error + data->pdata->first_point_trim;
 }
 
 static int exynos_tmu_initialize(struct platform_device *pdev)
@@ -311,8 +287,16 @@ static int exynos_tmu_read(struct exynos_tmu_data *data)
clk_enable(data->clk);
 
temp_code = readb(data->base + reg->tmu_cur_temp);
-   temp = code_to_temp(data, temp_code);
 
+   if (data->soc == SOC_ARCH_EXYNOS4210)
+   /* temp_code should range between 75 and 175 */
+   if (temp_code < 75 || temp_code > 175) {
+   temp = -ENODATA;
+   goto out;
+   }
+
+   temp = code_to_temp(data, temp_code);
+out:
clk_disable(data->clk);
mutex_unlock(&data->lock);
 
-- 
1.8.2.3

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

[PATCH v2 7/9] thermal: exynos: cache non_hw_trigger_levels in pdata

[Bartlomiej Zolnierkiewicz]

Cache number of non-hardware trigger levels in a new pdata field
(non_hw_trigger_levels) and convert code in exynos_tmu_initialize()
accordingly.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
Reviewed-by: Amit Daniel Kachhap 
---
 drivers/thermal/samsung/exynos_tmu.c  | 16 +++-
 drivers/thermal/samsung/exynos_tmu.h  |  2 ++
 drivers/thermal/samsung/exynos_tmu_data.c |  5 +
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 34ac081..9455b23 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -91,7 +91,7 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
const struct exynos_tmu_registers *reg = pdata->registers;
unsigned int status, trim_info = 0, con;
unsigned int rising_threshold = 0, falling_threshold = 0;
-   int ret = 0, threshold_code, i, trigger_levs = 0;
+   int ret = 0, threshold_code, i;
 
mutex_lock(&data->lock);
clk_enable(data->clk);
@@ -142,15 +142,6 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
data->temp_error > pdata->max_efuse_value)
data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
 
-   for (i = 0; i < pdata->max_trigger_level; i++) {
-   if (!pdata->trigger_levels[i])
-   continue;
-
-   /* Count trigger levels except the HW trip*/
-   if (!(pdata->trigger_type[i] == HW_TRIP))
-   trigger_levs++;
-   }
-
rising_threshold = readl(data->base + reg->threshold_th0);
 
if (data->soc == SOC_ARCH_EXYNOS4210) {
@@ -158,15 +149,14 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
threshold_code = temp_to_code(data, pdata->threshold);
writeb(threshold_code,
data->base + reg->threshold_temp);
-   for (i = 0; i < trigger_levs; i++)
+   for (i = 0; i < pdata->non_hw_trigger_levels; i++)
writeb(pdata->trigger_levels[i], data->base +
reg->threshold_th0 + i * sizeof(reg->threshold_th0));
 
writel(reg->intclr_rise_mask, data->base + reg->tmu_intclear);
} else {
/* Write temperature code for rising and falling threshold */
-   for (i = 0;
-   i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
+   for (i = 0; i < pdata->non_hw_trigger_levels; i++) {
threshold_code = temp_to_code(data,
pdata->trigger_levels[i]);
rising_threshold &= ~(0xff << 8 * i);
diff --git a/drivers/thermal/samsung/exynos_tmu.h 
b/drivers/thermal/samsung/exynos_tmu.h
index 60ecc61..b1c9f8d 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -186,6 +186,7 @@ struct exynos_tmu_registers {
  * 1 = enable trigger_level[] interrupt,
  * 0 = disable trigger_level[] interrupt
  * @max_trigger_level: max trigger level supported by the TMU
+ * @non_hw_trigger_levels: number of defined non-hardware trigger levels
  * @gain: gain of amplifier in the positive-TC generator block
  * 0 <= gain <= 15
  * @reference_voltage: reference voltage of amplifier
@@ -216,6 +217,7 @@ struct exynos_tmu_platform_data {
enum trigger_type trigger_type[MAX_TRIP_COUNT];
bool trigger_enable[MAX_TRIP_COUNT];
u8 max_trigger_level;
+   u8 non_hw_trigger_levels;
u8 gain;
u8 reference_voltage;
u8 noise_cancel_mode;
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c 
b/drivers/thermal/samsung/exynos_tmu_data.c
index 7a9cee5..4bc6b06 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -62,6 +62,7 @@ struct exynos_tmu_init_data const exynos4210_default_tmu_data 
= {
.trigger_type[1] = THROTTLE_ACTIVE,
.trigger_type[2] = SW_TRIP,
.max_trigger_level = 4,
+   .non_hw_trigger_levels = 3,
.gain = 15,
.reference_voltage = 7,
.min_efuse_value = 40,
@@ -135,6 +136,7 @@ static const struct exynos_tmu_registers 
exynos4412_tmu_registers = {
.trigger_type[2] = SW_TRIP, \
.trigger_type[3] = HW_TRIP, \
.max_trigger_level = 4, \
+   .non_hw_trigger_levels = 3, \
.gain = 8, \
.reference_voltage = 16, \
.noise_cancel_mode = 4, \
@@ -231,6 +233,7 @@ static const struct exynos_tmu_registers 
exynos5260_tmu_registers = {
.trigger_type[2] = SW_TRIP, \
.trigger_type[3] = HW_TRIP, \
.max_trigger_level = 4, \
+   .non_hw_trigger_levels = 3, \
.gain = 8, \
 

[PATCH v2 5/9] thermal: exynos: remove redundant threshold_code checks from exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Remove runtime checks for negative return values of temp_to_code()
from exynos_tmu_initialize().

The current level temperature data hardcoded in pdata will never
cause a negative temp_to_code() return values and checking itself
is not proper.  The checks in question are done at runtime in
a production code for data that is hardcoded inside driver during
development time and later it doesn't change.  Such data should
be verified during development and review time (i.e. by a script
parsing relevant data from exynos_tmu_data.c, one can also argue
that verification to be done is so simple that the review by
a maintainer should be enough).

Theres should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_tmu.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index f061580..204f811 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -180,10 +180,6 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
if (data->soc == SOC_ARCH_EXYNOS4210) {
/* Write temperature code for threshold */
threshold_code = temp_to_code(data, pdata->threshold);
-   if (threshold_code < 0) {
-   ret = threshold_code;
-   goto out;
-   }
writeb(threshold_code,
data->base + reg->threshold_temp);
for (i = 0; i < trigger_levs; i++)
@@ -197,19 +193,13 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
threshold_code = temp_to_code(data,
pdata->trigger_levels[i]);
-   if (threshold_code < 0) {
-   ret = threshold_code;
-   goto out;
-   }
rising_threshold &= ~(0xff << 8 * i);
rising_threshold |= threshold_code << 8 * i;
if (pdata->threshold_falling) {
threshold_code = temp_to_code(data,
pdata->trigger_levels[i] -
pdata->threshold_falling);
-   if (threshold_code > 0)
-   falling_threshold |=
-   threshold_code << 8 * i;
+   falling_threshold |= threshold_code << 8 * i;
}
}
 
@@ -228,10 +218,6 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
(pdata->trigger_type[i] == HW_TRIP)) {
threshold_code = temp_to_code(data,
pdata->trigger_levels[i]);
-   if (threshold_code < 0) {
-   ret = threshold_code;
-   goto out;
-   }
if (i == EXYNOS_MAX_TRIGGER_PER_REG - 1) {
/* 1-4 level to be assigned in th0 reg */
rising_threshold &= ~(0xff << 8 * i);
-- 
1.8.2.3

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

[PATCH v2 8/9] thermal: exynos: remove redundant pdata checks from exynos_tmu_control()

[Bartlomiej Zolnierkiewicz]

pdata->reference_voltage and pdata->gain are always defined
to non-zero values so remove the redundant checks from
exynos_tmu_control().

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_tmu.c | 12 
 drivers/thermal/samsung/exynos_tmu.h |  4 ++--
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 9455b23..b22f358 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -229,15 +229,11 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
if (pdata->test_mux)
con |= (pdata->test_mux << reg->test_mux_addr_shift);
 
-   if (pdata->reference_voltage) {
-   con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
-   con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
-   }
+   con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
+   con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
 
-   if (pdata->gain) {
-   con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
-   con |= (pdata->gain << reg->buf_slope_sel_shift);
-   }
+   con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
+   con |= (pdata->gain << reg->buf_slope_sel_shift);
 
if (pdata->noise_cancel_mode) {
con &= ~(reg->therm_trip_mode_mask <<
diff --git a/drivers/thermal/samsung/exynos_tmu.h 
b/drivers/thermal/samsung/exynos_tmu.h
index b1c9f8d..5d36708 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -188,10 +188,10 @@ struct exynos_tmu_registers {
  * @max_trigger_level: max trigger level supported by the TMU
  * @non_hw_trigger_levels: number of defined non-hardware trigger levels
  * @gain: gain of amplifier in the positive-TC generator block
- * 0 <= gain <= 15
+ * 0 < gain <= 15
  * @reference_voltage: reference voltage of amplifier
  * in the positive-TC generator block
- * 0 <= reference_voltage <= 31
+ * 0 < reference_voltage <= 31
  * @noise_cancel_mode: noise cancellation mode
  * 000, 100, 101, 110 and 111 can be different modes
  * @type: determines the type of SOC
-- 
1.8.2.3

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

[PATCH v2 3/9] thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING calibration

[Bartlomiej Zolnierkiewicz]

There are two types of calibration (TYPE_[ONE,TWO]_POINT_TRIMMING)
implemented in the driver currently, the other ones are defined in
calibration_type enum but are not implemented.

The commit 9d97e5c8 ("hwmon: Add driver for EXYNOS4 TMU") added
TYPE_TWO_POINT_TRIMMING implementation but no users of it have
ever been added.  Thus it has been a dead code for almost 3 years
now and should be removed.

We don't keep the unused/untested features in the kernel just
in case that some future hardware might need it.  Such code has
a real maintainance cost (all other code changes have to take
the dead code into account) and usually makes future changes
more difficult, not easier (i.e. recent additions of Exynos5420
SoC and Exynos5260 SoC thermal support has not made use of any
of the driver's currently unused/untested features, moreover
the recently added code is more complex than needed because of
the existing dead code).  Also all removed dead code is still
accessible in the kernel git repository and can be easily
brought back if/when needed.

This patch does TYPE_TWO_POINT_TRIMMING and related dead code
removals.

Please note that in exynos_tmu_initialize() if ->temp_error2 was
zero then its value was obtained from bits 8-15 of efuse_value
(bits 16-31 are always zero).  After TYPE_TWO_POINT_TRIMMING code
removal ->temp_error2 was no longer needed and was also removed.
Thus only bits 0-7 of efuse_value are ever used and its type can
be changed from u32 to u8.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_tmu.c  | 55 ++-
 drivers/thermal/samsung/exynos_tmu.h  | 20 +--
 drivers/thermal/samsung/exynos_tmu_data.c | 27 +--
 drivers/thermal/samsung/exynos_tmu_data.h |  2 --
 4 files changed, 12 insertions(+), 92 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 1050b36..1c64508 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -48,8 +48,7 @@
  * @lock: lock to implement synchronization.
  * @clk: pointer to the clock structure.
  * @clk_sec: pointer to the clock structure for accessing the base_second.
- * @temp_error1: fused value of the first point trim.
- * @temp_error2: fused value of the second point trim.
+ * @temp_error: fused value of the first point trim.
  * @regulator: pointer to the TMU regulator structure.
  * @reg_conf: pointer to structure to register with core thermal.
  */
@@ -63,14 +62,13 @@ struct exynos_tmu_data {
struct work_struct irq_work;
struct mutex lock;
struct clk *clk, *clk_sec;
-   u8 temp_error1, temp_error2;
+   u8 temp_error;
struct regulator *regulator;
struct thermal_sensor_conf *reg_conf;
 };
 
 /*
  * TMU treats temperature as a mapped temperature code.
- * The temperature is converted differently depending on the calibration type.
  */
 static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
 {
@@ -84,20 +82,7 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 
temp)
goto out;
}
 
-   switch (pdata->cal_type) {
-   case TYPE_TWO_POINT_TRIMMING:
-   temp_code = (temp - pdata->first_point_trim) *
-   (data->temp_error2 - data->temp_error1) /
-   (pdata->second_point_trim - pdata->first_point_trim) +
-   data->temp_error1;
-   break;
-   case TYPE_ONE_POINT_TRIMMING:
-   temp_code = temp + data->temp_error1 - pdata->first_point_trim;
-   break;
-   default:
-   temp_code = temp + pdata->default_temp_offset;
-   break;
-   }
+   temp_code = temp + data->temp_error - pdata->first_point_trim;
 out:
return temp_code;
 }
@@ -118,20 +103,7 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 
temp_code)
goto out;
}
 
-   switch (pdata->cal_type) {
-   case TYPE_TWO_POINT_TRIMMING:
-   temp = (temp_code - data->temp_error1) *
-   (pdata->second_point_trim - pdata->first_point_trim) /
-   (data->temp_error2 - data->temp_error1) +
-   pdata->first_point_trim;
-   break;
-   case TYPE_ONE_POINT_TRIMMING:
-   temp = temp_code - data->temp_error1 + pdata->first_point_trim;
-   break;
-   default:
-   temp = temp_code - pdata->default_temp_offset;
-   break;
-   }
+   temp = temp_code - data->temp_error + pdata->first_point_trim;
 out:
return temp;
 }
@@ -187,19 +159,12 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
else
trim_info = readl(data->base + reg->triminfo_data);
}
-   dat

[PATCH v2 9/9] thermal: exynos: remove identical values from exynos*_tmu_registers structures

[Bartlomiej Zolnierkiewicz]

There is no need for abstracting configuration for registers that
are identical on all SoC types.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
Reviewed-by: Amit Daniel Kachhap
---
 drivers/thermal/samsung/exynos_tmu.c  | 12 ++--
 drivers/thermal/samsung/exynos_tmu.h  | 11 ---
 drivers/thermal/samsung/exynos_tmu_data.c | 25 -
 3 files changed, 6 insertions(+), 42 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index b22f358..c47d2e2 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -229,11 +229,11 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
if (pdata->test_mux)
con |= (pdata->test_mux << reg->test_mux_addr_shift);
 
-   con &= ~(reg->buf_vref_sel_mask << reg->buf_vref_sel_shift);
-   con |= pdata->reference_voltage << reg->buf_vref_sel_shift;
+   con &= ~(EXYNOS_TMU_REF_VOLTAGE_MASK << EXYNOS_TMU_REF_VOLTAGE_SHIFT);
+   con |= pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIFT;
 
-   con &= ~(reg->buf_slope_sel_mask << reg->buf_slope_sel_shift);
-   con |= (pdata->gain << reg->buf_slope_sel_shift);
+   con &= ~(EXYNOS_TMU_BUF_SLOPE_SEL_MASK << 
EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
+   con |= (pdata->gain << EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT);
 
if (pdata->noise_cancel_mode) {
con &= ~(reg->therm_trip_mode_mask <<
@@ -242,7 +242,7 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
}
 
if (on) {
-   con |= (1 << reg->core_en_shift);
+   con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT);
interrupt_en =
pdata->trigger_enable[3] << reg->inten_rise3_shift |
pdata->trigger_enable[2] << reg->inten_rise2_shift |
@@ -252,7 +252,7 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
interrupt_en |=
interrupt_en << reg->inten_fall0_shift;
} else {
-   con &= ~(1 << reg->core_en_shift);
+   con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT);
interrupt_en = 0; /* Disable all interrupts */
}
writel(interrupt_en, data->base + reg->tmu_inten);
diff --git a/drivers/thermal/samsung/exynos_tmu.h 
b/drivers/thermal/samsung/exynos_tmu.h
index 5d36708..4f6f1b4 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -71,15 +71,9 @@ enum soc_type {
  * @triminfo_ctrl: trim info controller register.
  * @tmu_ctrl: TMU main controller register.
  * @test_mux_addr_shift: shift bits of test mux address.
- * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl register.
- * @buf_vref_sel_mask: mask bits of reference voltage in tmu_ctrl register.
  * @therm_trip_mode_shift: shift bits of tripping mode in tmu_ctrl register.
  * @therm_trip_mode_mask: mask bits of tripping mode in tmu_ctrl register.
  * @therm_trip_en_shift: shift bits of tripping enable in tmu_ctrl register.
- * @buf_slope_sel_shift: shift bits of amplifier gain value in tmu_ctrl
-   register.
- * @buf_slope_sel_mask: mask bits of amplifier gain value in tmu_ctrl register.
- * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
  * @tmu_status: register drescribing the TMU status.
  * @tmu_cur_temp: register containing the current temperature of the TMU.
  * @threshold_temp: register containing the base threshold level.
@@ -114,14 +108,9 @@ struct exynos_tmu_registers {
 
u32 tmu_ctrl;
u32 test_mux_addr_shift;
-   u32 buf_vref_sel_shift;
-   u32 buf_vref_sel_mask;
u32 therm_trip_mode_shift;
u32 therm_trip_mode_mask;
u32 therm_trip_en_shift;
-   u32 buf_slope_sel_shift;
-   u32 buf_slope_sel_mask;
-   u32 core_en_shift;
 
u32 tmu_status;
 
diff --git a/drivers/thermal/samsung/exynos_tmu_data.c 
b/drivers/thermal/samsung/exynos_tmu_data.c
index 4bc6b06..9dcf913 100644
--- a/drivers/thermal/samsung/exynos_tmu_data.c
+++ b/drivers/thermal/samsung/exynos_tmu_data.c
@@ -28,11 +28,6 @@
 static const struct exynos_tmu_registers exynos4210_tmu_registers = {
.triminfo_data = EXYNOS_TMU_REG_TRIMINFO,
.tmu_ctrl = EXYNOS_TMU_REG_CONTROL,
-   .buf_vref_sel_shift = EXYNOS_TMU_REF_VOLTAGE_SHIFT,
-   .buf_vref_sel_mask = EXYNOS_TMU_REF_VOLTAGE_MASK,
-   .buf_slope_sel_shift = EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT,
-   .buf_slope_sel_mask = EXYNOS_TMU_BUF_SLOPE_SEL_MASK,
-   .core_en_shift = EXYNOS_TMU_CORE_EN_SHIFT,
.tmu_status = EXYNOS_TMU_REG_STATUS,
.tmu_cur_temp = EXYNOS_TMU_REG_CURRENT_TEMP,
.threshold_temp = EXYNOS4210_TMU_REG_THRESHOLD_TEMP,
@@ -92,14 +87,9 @@ static 

[PATCH v2 4/9] thermal: exynos: remove redundant pdata checks from exynos_tmu_initialize()

[Bartlomiej Zolnierkiewicz]

Remove runtime checks for pdata sanity from exynos_tmu_initialize().

The current values hardcoded in pdata will never trigger the checks
and checking itself is not proper.  The checks in question are done
at runtime in a production code for data that is hardcoded inside
driver during development time and later it doesn't change.  Such
data should be verified during development and review time (i.e. by
a script parsing relevant data from exynos_tmu_data.c, one can also
argue that verification to be done is so simple that the review by
a maintainer should be enough).

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_thermal_common.h |  1 -
 drivers/thermal/samsung/exynos_tmu.c| 13 -
 2 files changed, 14 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_thermal_common.h 
b/drivers/thermal/samsung/exynos_thermal_common.h
index 3eb2ed9..cd44719 100644
--- a/drivers/thermal/samsung/exynos_thermal_common.h
+++ b/drivers/thermal/samsung/exynos_thermal_common.h
@@ -27,7 +27,6 @@
 #define SENSOR_NAME_LEN16
 #define MAX_TRIP_COUNT 8
 #define MAX_COOLING_DEVICE 4
-#define MAX_THRESHOLD_LEVS 5
 
 #define ACTIVE_INTERVAL 500
 #define IDLE_INTERVAL 1
diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index 1c64508..f061580 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -166,23 +166,10 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
data->temp_error > pdata->max_efuse_value)
data->temp_error = pdata->efuse_value & EXYNOS_TMU_TEMP_MASK;
 
-   if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
-   dev_err(&pdev->dev, "Invalid max trigger level\n");
-   ret = -EINVAL;
-   goto out;
-   }
-
for (i = 0; i < pdata->max_trigger_level; i++) {
if (!pdata->trigger_levels[i])
continue;
 
-   if ((pdata->trigger_type[i] == HW_TRIP) &&
-   (!pdata->trigger_levels[pdata->max_trigger_level - 1])) {
-   dev_err(&pdev->dev, "Invalid hw trigger level\n");
-   ret = -EINVAL;
-   goto out;
-   }
-
/* Count trigger levels except the HW trip*/
if (!(pdata->trigger_type[i] == HW_TRIP))
trigger_levs++;
-- 
1.8.2.3

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

[PATCH v2 2/9] thermal: exynos: remove dead code for HW_MODE calibration

[Bartlomiej Zolnierkiewicz]

The commit 1928457 ("thermal: exynos: Add hardware mode thermal
calibration support") has added HW_MODE feature but it has never
been enabled.  As such it has been a dead code for almost a year
now and should be removed from the kernel.

We don't keep the unused/untested features in the kernel just
in case that some future hardware might need it.  Such code has
a real maintainance cost (all other code changes have to take
the dead code into account) and usually makes future changes
more difficult, not easier (i.e. recent additions of Exynos5420
SoC and Exynos5260 SoC thermal support has not made use of any
of the driver's currently unused/untested features, moreover
the recently added code is more complex than needed because of
the existing dead code).  Also all removed dead code is still
accessible in the kernel git repository and can be easily
brought back if/when needed.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_tmu.c  | 33 +--
 drivers/thermal/samsung/exynos_tmu.h  | 13 
 drivers/thermal/samsung/exynos_tmu_data.c |  3 ---
 drivers/thermal/samsung/exynos_tmu_data.h |  2 --
 4 files changed, 1 insertion(+), 50 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.c 
b/drivers/thermal/samsung/exynos_tmu.c
index d7ca9f4..1050b36 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -77,9 +77,6 @@ static int temp_to_code(struct exynos_tmu_data *data, u8 temp)
struct exynos_tmu_platform_data *pdata = data->pdata;
int temp_code;
 
-   if (pdata->cal_mode == HW_MODE)
-   return temp;
-
if (data->soc == SOC_ARCH_EXYNOS4210)
/* temp should range between 25 and 125 */
if (temp < 25 || temp > 125) {
@@ -114,9 +111,6 @@ static int code_to_temp(struct exynos_tmu_data *data, u8 
temp_code)
struct exynos_tmu_platform_data *pdata = data->pdata;
int temp;
 
-   if (pdata->cal_mode == HW_MODE)
-   return temp_code;
-
if (data->soc == SOC_ARCH_EXYNOS4210)
/* temp_code should range between 75 and 175 */
if (temp_code < 75 || temp_code > 175) {
@@ -167,9 +161,6 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
if (TMU_SUPPORTS(pdata, TRIM_RELOAD))
__raw_writel(1, data->base + reg->triminfo_ctrl);
 
-   if (pdata->cal_mode == HW_MODE)
-   goto skip_calib_data;
-
/* Save trimming info in order to perform calibration */
if (data->soc == SOC_ARCH_EXYNOS5440) {
/*
@@ -210,7 +201,6 @@ static int exynos_tmu_initialize(struct platform_device 
*pdev)
(pdata->efuse_value >> reg->triminfo_85_shift) &
EXYNOS_TMU_TEMP_MASK;
 
-skip_calib_data:
if (pdata->max_trigger_level > MAX_THRESHOLD_LEVS) {
dev_err(&pdev->dev, "Invalid max trigger level\n");
ret = -EINVAL;
@@ -325,7 +315,7 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
struct exynos_tmu_data *data = platform_get_drvdata(pdev);
struct exynos_tmu_platform_data *pdata = data->pdata;
const struct exynos_tmu_registers *reg = pdata->registers;
-   unsigned int con, interrupt_en, cal_val;
+   unsigned int con, interrupt_en;
 
mutex_lock(&data->lock);
clk_enable(data->clk);
@@ -351,27 +341,6 @@ static void exynos_tmu_control(struct platform_device 
*pdev, bool on)
con |= (pdata->noise_cancel_mode << reg->therm_trip_mode_shift);
}
 
-   if (pdata->cal_mode == HW_MODE) {
-   con &= ~(reg->calib_mode_mask << reg->calib_mode_shift);
-   cal_val = 0;
-   switch (pdata->cal_type) {
-   case TYPE_TWO_POINT_TRIMMING:
-   cal_val = 3;
-   break;
-   case TYPE_ONE_POINT_TRIMMING_85:
-   cal_val = 2;
-   break;
-   case TYPE_ONE_POINT_TRIMMING_25:
-   cal_val = 1;
-   break;
-   case TYPE_NONE:
-   break;
-   default:
-   dev_err(&pdev->dev, "Invalid calibration type, using 
none\n");
-   }
-   con |= cal_val << reg->calib_mode_shift;
-   }
-
if (on) {
con |= (1 << reg->core_en_shift);
interrupt_en =
diff --git a/drivers/thermal/samsung/exynos_tmu.h 
b/drivers/thermal/samsung/exynos_tmu.h
index 59799ef..86c5531 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -34,11 +34,6 @@ enum calibration_type {
TYPE_NONE,
 };
 
-enum calibration_mode {
-   SW_MODE,
-   HW_MODE,
-};
-
 enum soc_type {

[PATCH v2 0/9] thermal: exynos: various cleanups

[Bartlomiej Zolnierkiewicz]

Hi,

This patch series contains various cleanups for EXYNOS thermal
driver.  Overall it decreases driver's LOC by 12%.  It is based
on next-20140617 kernel.  It should not cause any functionality
changes.

Changes since v1:
- synced patches against next-20140617
- merged patch "thermal: exynos: remove unused defines" into
  "thermal: exynos: remove unused struct exynos_tmu_registers
  entries" one (per request from Eduardo)
- improved patch descriptions for patches #1-5
- fixed documentation for pdata->gain and pdata->reference_voltage
- added Reviewed-by from Amit to patches #6, #7 and #10
- added missing Acked-by from Kyungmin Park

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics


Bartlomiej Zolnierkiewicz (9):
  thermal: exynos: remove unused struct exynos_tmu_registers entries
  thermal: exynos: remove dead code for HW_MODE calibration
  thermal: exynos: remove dead code for TYPE_TWO_POINT_TRIMMING
calibration
  thermal: exynos: remove redundant pdata checks from
exynos_tmu_initialize()
  thermal: exynos: remove redundant threshold_code checks from
exynos_tmu_initialize()
  thermal: exynos: simplify temp_to_code() and code_to_temp()
  thermal: exynos: cache non_hw_trigger_levels in pdata
  thermal: exynos: remove redundant pdata checks from
exynos_tmu_control()
  thermal: exynos: remove identical values from exynos*_tmu_registers
structures

 drivers/thermal/samsung/exynos_thermal_common.h |   1 -
 drivers/thermal/samsung/exynos_tmu.c| 181 
 drivers/thermal/samsung/exynos_tmu.h|  90 +---
 drivers/thermal/samsung/exynos_tmu_data.c   |  64 +
 drivers/thermal/samsung/exynos_tmu_data.h   |  33 +
 5 files changed, 41 insertions(+), 328 deletions(-)

-- 
1.8.2.3

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

[PATCH v2 1/9] thermal: exynos: remove unused struct exynos_tmu_registers entries

[Bartlomiej Zolnierkiewicz]

Remove unused / write-only entries from struct exynos_tmu_registers.
Then remove unused defines while at it.

We don't keep the unused/untested features in the kernel just
in case that some future hardware might need it.  Such code has
a real maintainance cost (all other code changes have to take
the dead code into account) and usually makes future changes
more difficult, not easier (i.e. recent additions of Exynos5420
SoC and Exynos5260 SoC thermal support has not made use of any
of the driver's currently unused/untested features, moreover
the recently added code is more complex than needed because of
the existing dead code).  Also all removed dead code is still
accessible in the kernel git repository and can be easily
brought back if/when needed.

There should be no functional changes caused by this patch.

Signed-off-by: Bartlomiej Zolnierkiewicz 
Acked-by: Kyungmin Park 
---
 drivers/thermal/samsung/exynos_tmu.h  | 40 ---
 drivers/thermal/samsung/exynos_tmu_data.c |  4 
 drivers/thermal/samsung/exynos_tmu_data.h | 29 +-
 3 files changed, 1 insertion(+), 72 deletions(-)

diff --git a/drivers/thermal/samsung/exynos_tmu.h 
b/drivers/thermal/samsung/exynos_tmu.h
index edd08cf..59799ef 100644
--- a/drivers/thermal/samsung/exynos_tmu.h
+++ b/drivers/thermal/samsung/exynos_tmu.h
@@ -84,8 +84,6 @@ enum soc_type {
  * @triminfo_25_shift: shift bit of the 25 C trim value in triminfo_data reg.
  * @triminfo_85_shift: shift bit of the 85 C trim value in triminfo_data reg.
  * @triminfo_ctrl: trim info controller register.
- * @triminfo_reload_shift: shift of triminfo reload enable bit in triminfo_ctrl
-   reg.
  * @tmu_ctrl: TMU main controller register.
  * @test_mux_addr_shift: shift bits of test mux address.
  * @buf_vref_sel_shift: shift bits of reference voltage in tmu_ctrl register.
@@ -100,27 +98,13 @@ enum soc_type {
register.
  * @calib_mode_mask: mask bits of calibration mode value in tmu_ctrl
register.
- * @therm_trip_tq_en_shift: shift bits of thermal trip enable by TQ pin in
-   tmu_ctrl register.
  * @core_en_shift: shift bits of TMU core enable bit in tmu_ctrl register.
  * @tmu_status: register drescribing the TMU status.
  * @tmu_cur_temp: register containing the current temperature of the TMU.
- * @tmu_cur_temp_shift: shift bits of current temp value in tmu_cur_temp
-   register.
  * @threshold_temp: register containing the base threshold level.
  * @threshold_th0: Register containing first set of rising levels.
- * @threshold_th0_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th0_l1_shift: shift bits of level1 threshold temperature.
- * @threshold_th0_l2_shift: shift bits of level2 threshold temperature.
- * @threshold_th0_l3_shift: shift bits of level3 threshold temperature.
  * @threshold_th1: Register containing second set of rising levels.
- * @threshold_th1_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th1_l1_shift: shift bits of level1 threshold temperature.
- * @threshold_th1_l2_shift: shift bits of level2 threshold temperature.
- * @threshold_th1_l3_shift: shift bits of level3 threshold temperature.
  * @threshold_th2: Register containing third set of rising levels.
- * @threshold_th2_l0_shift: shift bits of level0 threshold temperature.
- * @threshold_th3: Register containing fourth set of rising levels.
  * @threshold_th3_l0_shift: shift bits of level0 threshold temperature.
  * @tmu_inten: register containing the different threshold interrupt
enable bits.
@@ -129,9 +113,6 @@ enum soc_type {
  * @inten_rise2_shift: shift bits of rising 2 interrupt bits.
  * @inten_rise3_shift: shift bits of rising 3 interrupt bits.
  * @inten_fall0_shift: shift bits of falling 0 interrupt bits.
- * @inten_fall1_shift: shift bits of falling 1 interrupt bits.
- * @inten_fall2_shift: shift bits of falling 2 interrupt bits.
- * @inten_fall3_shift: shift bits of falling 3 interrupt bits.
  * @tmu_intstat: Register containing the interrupt status values.
  * @tmu_intclear: Register for clearing the raised interrupt status.
  * @intclr_fall_shift: shift bits for interrupt clear fall 0
@@ -141,7 +122,6 @@ enum soc_type {
  * @emul_con: TMU emulation controller register.
  * @emul_temp_shift: shift bits of emulation temperature.
  * @emul_time_shift: shift bits of emulation time.
- * @emul_time_mask: mask bits of emulation time.
  * @tmu_irqstatus: register to find which TMU generated interrupts.
  * @tmu_pmin: register to get/set the Pmin value.
  */
@@ -152,7 +132,6 @@ struct exynos_tmu_registers {
 
u32 triminfo_ctrl;
u32 triminfo_ctrl1;
-   u32 triminfo_reload_shift;
 
u32 tmu_ctrl;
u32 test_mux_addr_shift;
@@ -165,32 +144,17 @@ struct exynos_tmu_registers {
u32 buf_slope_sel_mask;
u32 calib_mode_shift;
u32 calib_mode_mask;
-   u32 therm_trip_tq_en_shift;
u32 cor

Re: [PATCH v4 10/11] ARM: EXYNOS: Add platform driver support for Exynos PMU.

[Tomasz Figa]

Hi Pankaj,

[dropping Young-Gun Jang , as my mailer denies
to send to this address]

Please see my comments inline. I have skipped comments for changed
related to regmap, as according to our previous discussion they will be
dropped in next version.

On 10.05.2014 08:56, Pankaj Dubey wrote:
> This patch modifies Exynos Power Management Unit (PMU) initialization
> implementation in following way:
> 
> - Added platform_device support by registering static platform device.
> - Added platform struct exynos_pmu_data to hold platform specific data.
> - For each SoC's PMU support now we can add platform data and statically
>   bind PMU configuration and SoC specific initialization function.
> - Probe function will scan DT and based on matching PMU compatibility
>   string initialize pmu_context which will be platform_data for driver.
> - Obtain PMU regmap handle using "syscon_regmap_lookup_by_phandle" so
>   that we can reduce dependency over machine header files.
> - Separate each SoC's PMU initialization function and make it as part of
>   platform data.
> - It also removes uses of soc_is_exynosXYZ() thus making PMU implementation
>   independent of "plat/cpu.h".
> 
> Signed-off-by: Pankaj Dubey 
> Signed-off-by: Young-Gun Jang 
> ---
>  arch/arm/mach-exynos/pmu.c |  266 
> ++--
>  1 file changed, 210 insertions(+), 56 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index 67116a5..6a7fa8e 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2011-2012 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2011-2014 Samsung Electronics Co., Ltd.
>   *   http://www.samsung.com/
>   *
>   * EXYNOS - CPU PMU(Power Management Unit) support
> @@ -9,20 +9,33 @@
>   * published by the Free Software Foundation.
>   */
>  
> -#include 
> -#include 
> +#include 
>  #include 
> -
> -#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  #include "common.h"
>  #include "regs-pmu.h"
>  
> -static const struct exynos_pmu_conf *exynos_pmu_config;
> -static struct regmap *pmu_regmap;
> +struct exynos_pmu_data {
> + const struct exynos_pmu_conf *pmu_config;
> + const struct exynos_pmu_conf *pmu_config_extra;
> + void (*pmu_init)(void);
> + void (*powerdown_conf)(enum sys_powerdown);
> +};
> +
> +struct exynos_pmu_context {
> + struct device *dev;
> + struct exynos_pmu_data *pmu_data;
> + struct regmap   *pmu_regmap;
> +};
> +
> +static struct exynos_pmu_context *pmu_context;
>  
>  static const struct exynos_pmu_conf exynos4210_pmu_config[] = {
> - /* { .reg = address, .val = { AFTR, LPA, SLEEP } */
> + /* { .offset = address, .val = { AFTR, LPA, SLEEP } */

AFAICT those have changed already in patch 08/11.

>   { S5P_ARM_CORE0_LOWPWR, { 0x0, 0x0, 0x2 } },
>   { S5P_DIS_IRQ_CORE0,{ 0x0, 0x0, 0x0 } },
>   { S5P_DIS_IRQ_CENTRAL0, { 0x0, 0x0, 0x0 } },
> @@ -216,7 +229,7 @@ static const struct exynos_pmu_conf 
> exynos4412_pmu_config[] = {
>  };
>  
>  static const struct exynos_pmu_conf exynos5250_pmu_config[] = {
> - /* { .reg = address, .val = { AFTR, LPA, SLEEP } */
> + /* { .offset = address, .val = { AFTR, LPA, SLEEP } */

Ditto.

>   { EXYNOS5_ARM_CORE0_SYS_PWR_REG,{ 0x0, 0x0, 0x2} },
>   { EXYNOS5_DIS_IRQ_ARM_CORE0_LOCAL_SYS_PWR_REG,  { 0x0, 0x0, 0x0} },
>   { EXYNOS5_DIS_IRQ_ARM_CORE0_CENTRAL_SYS_PWR_REG,{ 0x0, 0x0, 
> 0x0} },
> @@ -339,7 +352,7 @@ static unsigned int const exynos5_list_diable_wfi_wfe[] = 
> {
>   EXYNOS5_ISP_ARM_OPTION,
>  };
>  
> -static void exynos5_init_pmu(void)
> +void exynos5_powerdown_conf(enum sys_powerdown mode)

static

>  {
>   unsigned int i;
>   unsigned int tmp;
> @@ -348,81 +361,222 @@ static void exynos5_init_pmu(void)
>* Enable both SC_FEEDBACK and SC_COUNTER
>*/
>   for (i = 0 ; i < ARRAY_SIZE(exynos5_list_both_cnt_feed) ; i++) {
> - regmap_read(pmu_regmap, exynos5_list_both_cnt_feed[i], &tmp);
> + regmap_read(pmu_context->pmu_regmap,
> + exynos5_list_both_cnt_feed[i], &tmp);
>   tmp |= (EXYNOS5_USE_SC_FEEDBACK |
>   EXYNOS5_USE_SC_COUNTER);
> - regmap_write(pmu_regmap, exynos5_list_both_cnt_feed[i], tmp);
> + regmap_write(pmu_context->pmu_regmap,
> + exynos5_list_both_cnt_feed[i], tmp);
>   }
>  
>   /*
>* SKIP_DEACTIVATE_ACEACP_IN_PWDN_BITFIELD Enable
>*/
> - regmap_read(pmu_regmap, EXYNOS5_ARM_COMMON_OPTION, &tmp);
> + regmap_read(pmu_context->pmu_regmap, EXYNOS5_ARM_COMMON_OPTION, &tmp);
>   tmp |= EXYNOS5_SKIP_DEACTIVATE_ACEACP_IN_PWDN;
> - regmap_write(pmu_regmap, EXYNOS5_ARM_COMMON_OPTION, tmp);
> + regmap_write(pmu_context->pmu_regmap, EXYNOS5_ARM_COMMON_OPTION, tmp

Re: [PATCH v3 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Paul Bolle]

On Tue, 2014-06-17 at 10:20 -0600, Stephen Warren wrote:
> On 06/17/2014 02:53 AM, Paul Bolle wrote:
> > So, in summary, while we're apparently only discussing a single comment,
> > I would appreciate it if it could be reworded, preferably by dropping
> > that the CONFIG_ prefix. But other people might care very little, as
> > they don't share this particular pet peeve.
> 
> Can't your tool maintain a whitelist or ignore list?

Sure it can. But I do think I should try to fix the (in my view, at
least) problems I find before adding stuff to a whitelist or (whatever).

> There are many
> cases where the kernel can pull in headers/data from other projects
> (Firmware interfaces to an arbitrarily large set of HW, Device trees,
> IO/network protocools, perhaps more). It feels quite unreasonable for
> the kernel to decide that it exclusively owns the CONFIG_* namespace
> even in comments, and that every other project it interacts with must
> not use that namespace.

As I said, this is more my peeve. Then again, referring to a macro from
some other project is likely to confuse people.


Paul Bolle

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

Re: [PATCH 2/2] ARM: dts: Add cros_ec to exynos5420-peach-pit and exynos5800-peach-pi

[Javier Martinez Canillas]

On 06/13/2014 08:13 PM, Doug Anderson wrote:
> This adds cros_ec to exynos5420-peach-pit and exynos5800-peach-pi,
> including:
> * The keyboard
> * The i2c tunnel
> * The tps65090 under the i2c tunnel
> * The battery under the i2c tunnel
> 
> To add extra motivation, it should be noted that tps65090 is one of
> the things needed to get display-related FETs turned on for pit and
> pi.
> 
> Note that this relies on a few outstanding changes:
> * Needs "cros-ec-keyboard.dtsi" in order to compile properly.  See
>   (ARM: dts: Create a cros-ec-keyboard fragment) at
>   .
> * Needs (mfd: cros_ec: spi: Fix end of transfer on devices with no
>   spi-msg-delay) from this series to work properly.
> * Needs (spi: s3c64xx: fix broken "cs_gpios" usage in the driver) and
>   (spi: s3c64xx: for DT platofrms always get the chipselect info from
>   DT node) to work properly and match the documented bindings.  See
>    and
>   
> 
> Signed-off-by: Doug Anderson 
> ---
>  arch/arm/boot/dts/exynos5420-peach-pit.dts | 146 
> +
>  arch/arm/boot/dts/exynos5800-peach-pi.dts  | 146 
> +
>  2 files changed, 292 insertions(+)
> 

After applying all the dependencies and this patch, I see that cros-ec-spi
driver is probed and also the keyboard is working on my Peach pit when testing
with: $ evtest /dev/input/event0

Tested-by: Javier Martinez Canillas 

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 v3 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Stephen Warren]

On 06/17/2014 02:53 AM, Paul Bolle wrote:
> Doug,
> 
> On Fri, 2014-06-13 at 08:22 -0700, Doug Anderson wrote:
>> On Fri, Jun 13, 2014 at 1:08 AM, Paul Bolle  wrote:
>>> On Wed, 2014-06-11 at 08:11 -0700, Doug Anderson wrote:
 This is a config option on the ChromeOS EC
 .  Doing a
 grep there:

 board/samus/board.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
 common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
 common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
 common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
 driver/battery/samus.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
 driver/battery/samus.c:#endif   /* CONFIG_CHARGER_PROFILE_OVERRIDE */
 include/config.h:#undef CONFIG_CHARGER_PROFILE_OVERRIDE
 include/ec_commands.h:  /* Range for CONFIG_CHARGER_PROFILE_OVERRIDE 
 params */
 test/test_config.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
>>>
>>> I see. So this is not a Kconfig macro but a general macro with a CONFIG_
>>> prefix. There are quite a bit of those in the tree already, but still,
>>> would another prefix also do?
>>
>> Given that it's an entirely separate project and this is a valid
>> CONFIG option in that project, it seems a lot to ask them not to use
>> the CONFIG_ prefix.  Also: the part you are objecting to is only a
>> comment, right?
> 
> So all the hits you quoted above are actually from code that's never
> going to be included in the kernel tree, right? If so, then yes, we're
> only discussing a single comment.
> 
>> We could certainly add extra wording in the comment to make it obvious
>> that this is a CONFIG option for the EC and not the kernel.  Would
>> that be enough?  ...or are you trying to use some scripts to
>> automatically process files to look for CONFIG options?
> 
> Yes, I'm using a script to check for Kconfig macros, among other things.
> It doesn't care about comments (because every now and then mistakes are
> made in comments too, and some of those can get surprisingly confusing).
> 
> Anyhow, the CONFIG_ prefix used in the kernel tree is quite generic, but
> we're stuck with it. Would it be bothersome to drop it in that comment?
> Mentioning a preprocessor macro from a separate project is a bit
> confusing to begin with. How is one supposed to know that this is a
> reference to something out of tree?
> 
> So, in summary, while we're apparently only discussing a single comment,
> I would appreciate it if it could be reworded, preferably by dropping
> that the CONFIG_ prefix. But other people might care very little, as
> they don't share this particular pet peeve.

Can't your tool maintain a whitelist or ignore list? There are many
cases where the kernel can pull in headers/data from other projects
(Firmware interfaces to an arbitrarily large set of HW, Device trees,
IO/network protocools, perhaps more). It feels quite unreasonable for
the kernel to decide that it exclusively owns the CONFIG_* namespace
even in comments, and that every other project it interacts with must
not use that namespace.
--
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 07/10] regulator: Add driver for Maxim 77802 PMIC regulators

[Javier Martinez Canillas]

Hello Mark,

On 06/17/2014 04:12 PM, Mark Brown wrote:
> On Tue, Jun 17, 2014 at 12:49:56PM +0200, Javier Martinez Canillas wrote:
>> On 06/16/2014 09:25 PM, Mark Brown wrote:
> 
>> >> + config.dev = &pdev->dev;
> 
>> > Are you sure this shouldn't be the MFD?
> 
>> I just looked at regulator_register() and saw that it does rdev->dev.parent =
>> dev, so yes this has to be the MFD.
>

I noticed that many drivers set config.dev = &pdev->dev. The original Chrome OS
max77xxx driver and max77686 are two examples but others drivers do the same:

$ git grep "config.dev = &pdev->dev" drivers/regulator/ | wc -l
35
$ git grep "config.dev = pdev->dev.parent" drivers/regulator/ | wc -l
11

And also I see that mfd_add_device() calls
devm_regulator_bulk_register_supply_alias(&pdev->dev,...) so I'm confused now
about what the correct device should be...

> Do the regulators manage to get their supplies?
> 

There are no current support in mainline for the devices that use the regulators
in this PMIC so I can't tell you if consumers manage to get their supplies
correctly (e.g: if regulator_dev_lookup succeeds).

But I see in the kernel log that the regulators are registered and configured as
expected [0] and also the driver in the Chrome OS 3.8 kernel is working for sure
and sets config.dev to &pdev->dev instead of the MFD.

>> So, for now I thought it made sense to set the operating mode to normal on
>> probe() but I'll change it to read from the hardware if that is better.
> 
> Yes, otherwise if the device is configured otherwise then when we change
> the configuration we may break something.
> 
>> I guess I should check in the datasheet if a sane default operating mode for
>> LDOs is expected when the chip is reseted or if this is left undefined and 
>> also
>> if the bootloader already set this.
> 
> You can't do anything based on the particular bootloader you're using in
> your current system, this has to work in other systems.
> 

Yes, that's why I thought it was a good idea to set to a default operational
mode but I'll change it to read from the hardware instead.

Thanks a lot and best regards,
Javier

[0]: http://pastebin.com/raw.php?i=8yyMXcGD
--
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 09/11] ARM: EXYNOS: Move "mach/map.h" inclusion from regs-pmu.h to platsmp.c

[Tomasz Figa]

On 10.05.2014 08:56, Pankaj Dubey wrote:
> As we have removed static mappings from "regs-pmu.h" it does not
> need map.h anymore. But "platsmp.c" needed this and till now it
> got included indirectly. So lets move header inclusion of
> "mach/map.h" from "regs-pmu.h" to "platsmp.c".
> 
> Signed-off-by: Pankaj Dubey 
> ---
>  arch/arm/mach-exynos/platsmp.c  |1 +
>  arch/arm/mach-exynos/regs-pmu.h |2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)

This could be probably squashed with other one of previous patches, but
anyway:

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 v4 08/11] ARM: EXYNOS: Refactored code for using PMU address via DT

[Tomasz Figa]

Hi Pankaj,

[Dropped yg1004.j...@samsung.com from CC, as apparently his mailbox is
"temporarily locked", at least from the point of view of my mail server.]

Please see my comments inline.

On 10.05.2014 08:56, Pankaj Dubey wrote:
> Under "arm/mach-exynos" many files are using PMU register offsets.
> Since we have added support for accessing PMU base address via DT,
> now we can remove PMU mapping from exynosX_iodesc. Let's convert
> all these access using either of iomapped address or regmap handle.
> This will help us in removing static mapping of PMU base address
> as well as help in reducing dependency over machine header files.
> Thus helping for migration of PMU implementation from machine to
> driver folder which can be reused for ARM64 bsed SoC.

[snip]

> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 3b1d245..59eb1f1 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c

[snip]

> @@ -156,7 +147,7 @@ static void exynos_restart(enum reboot_mode mode, const 
> char *cmd)
>  {
>   struct device_node *np;
>   u32 val = 0x1;
> - void __iomem *addr = EXYNOS_SWRESET;
> + void __iomem *addr = NULL;

This will probably change into addr = exynos_pmu_base + EXYNOS_SWRESET.

>  
>   if (of_machine_is_compatible("samsung,exynos5440")) {
>   u32 status;
> @@ -169,9 +160,9 @@ static void exynos_restart(enum reboot_mode mode, const 
> char *cmd)
>   val = __raw_readl(addr);
>  
>   val = (val & 0x) | (status & 0x);
> - }
> -
> - __raw_writel(val, addr);
> + __raw_writel(val, addr);
> + } else
> + regmap_write(exynos_pmu_regmap, EXYNOS_SWRESET, val);
>  }
>  
>  static struct platform_device exynos_cpuidle = {
> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> index 73b0b5c..0243ef3 100644
> --- a/arch/arm/mach-exynos/hotplug.c
> +++ b/arch/arm/mach-exynos/hotplug.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -91,11 +92,12 @@ static inline void cpu_leave_lowpower(void)
>  
>  static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
>  {
> + struct regmap *pmu_regmap = get_exynos_pmuregmap();
>   for (;;) {
>  
>   /* make cpu1 to be turned off at next WFI command */
>   if (cpu == 1)
> - __raw_writel(0, S5P_ARM_CORE1_CONFIGURATION);
> + regmap_write(pmu_regmap, S5P_ARM_CORE1_CONFIGURATION, 
> 0);

This change should disappear.

[snip]

> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 1f63596..33eb364 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c

[snip]

> +static void __iomem *pmu_base_addr(void)
> +{
> + struct device_node *np = NULL;
> + if (!pmu_base) {
> + np = of_find_matching_node(NULL, exynos_dt_pmu_match);
> +
> + if (!np)
> + panic("%s, failed to find PMU node\n", __func__);
> +
> + pmu_base = of_iomap(np, 0);
> +
> + if (!pmu_base)
> + panic("%s: failed to map registers\n", __func__);
> + }
> + return pmu_base;
> +}

As we discussed before, PMU mapping for arch code should be handled in
the same way as SYSRAM.

>  static DEFINE_SPINLOCK(boot_lock);
>  
>  static void exynos_secondary_init(unsigned int cpu)
> @@ -132,14 +158,14 @@ static int exynos_boot_secondary(unsigned int cpu, 
> struct task_struct *idle)
>*/
>   write_pen_release(phys_cpu);
>  
> - if (!(__raw_readl(S5P_ARM_CORE1_STATUS) & S5P_CORE_LOCAL_PWR_EN)) {
> + if (!(__raw_readl(pmu_base + S5P_ARM_CORE1_STATUS)
> + & S5P_CORE_LOCAL_PWR_EN)) {

Creating a variable to save register value to would probably make the
code more readable.

>   __raw_writel(S5P_CORE_LOCAL_PWR_EN,
> -  S5P_ARM_CORE1_CONFIGURATION);
> -
> + pmu_base + S5P_ARM_CORE1_CONFIGURATION);
>   timeout = 10;
>  
>   /* wait max 10 ms until cpu1 is on */
> - while ((__raw_readl(S5P_ARM_CORE1_STATUS)
> + while ((__raw_readl(pmu_base + S5P_ARM_CORE1_STATUS)
>   & S5P_CORE_LOCAL_PWR_EN) != S5P_CORE_LOCAL_PWR_EN) {

Ditto.

>   if (timeout-- == 0)
>   break;
> @@ -238,6 +264,8 @@ static void __init exynos_smp_prepare_cpus(unsigned int 
> max_cpus)
>  {
>   int i;
>  
> + pmu_base = pmu_base_addr();
> +
>   if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
>   scu_enable(scu_base_addr());
>  
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index f445c49..ee427d7 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 

Re: [PATCH RFC] mfd: syscon: Decouple syscon interface from syscon devices

[Arnd Bergmann]

On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
> Currently a syscon entity can be only registered directly through a
> platform device that binds to a dedicated driver. However in certain use
> cases it is desirable to make a device used with another driver a syscon
> interface provider. For example, certain SoCs (e.g. Exynos) contain
> system controller blocks which perform various functions such as power
> domain control, CPU power management, low power mode control, but in
> addition contain certain IP integration glue, such as various signal
> masks, coprocessor power control, etc. In such case, there is a need to
> have a dedicated driver for such system controller but also share
> registers with other drivers. The latter is where the syscon interface
> is helpful.
> 
> This patch decouples syscon object from syscon driver, so that it can be
> registered from any driver in addition to the original "syscon" platform
> driver.
> 
> Signed-off-by: Tomasz Figa 

Hi Tomasz,

This seems like a reasonable way of solving the problem, but I think
there is an even better one that we have about in the past: if we
promote syscon from a platform driver into a a drivers/base/ helper
that is independent of the platform device matching, we can use
call syscon_regmap_lookup_* for any device node, whether it's already
bound to a driver or not, which do what you need. It would also make
it easier to call the syscon code before the platform_device
infrastructure gets initialized, which is something a number of
people have asked for, e.g. for using regmap to do SMP bringup
or for clock registration.

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

[PATCH v6 6/6] cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support

[Thomas Abraham]

From: Thomas Abraham 

Exynos4210 and Exynos5250 based platforms have switched over to use generic
cpufreq drivers for cpufreq functionality. So the Exynos specific cpufreq
drivers for these platforms can be removed.

Cc: Viresh Kumar 
Signed-off-by: Thomas Abraham 
---
 drivers/cpufreq/Kconfig.arm  |   22 
 drivers/cpufreq/Makefile |2 -
 drivers/cpufreq/exynos4210-cpufreq.c |  184 -
 drivers/cpufreq/exynos5250-cpufreq.c |  210 --
 4 files changed, 418 deletions(-)
 delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
 delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index ebac671..7a2f289 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -28,17 +28,6 @@ config ARM_VEXPRESS_SPC_CPUFREQ
 config ARM_EXYNOS_CPUFREQ
bool
 
-config ARM_EXYNOS4210_CPUFREQ
-   bool "SAMSUNG EXYNOS4210"
-   depends on CPU_EXYNOS4210
-   default y
-   select ARM_EXYNOS_CPUFREQ
-   help
- This adds the CPUFreq driver for Samsung EXYNOS4210
- SoC (S5PV310 or S5PC210).
-
- If in doubt, say N.
-
 config ARM_EXYNOS4X12_CPUFREQ
bool "SAMSUNG EXYNOS4x12"
depends on SOC_EXYNOS4212 || SOC_EXYNOS4412
@@ -50,17 +39,6 @@ config ARM_EXYNOS4X12_CPUFREQ
 
  If in doubt, say N.
 
-config ARM_EXYNOS5250_CPUFREQ
-   bool "SAMSUNG EXYNOS5250"
-   depends on SOC_EXYNOS5250
-   default y
-   select ARM_EXYNOS_CPUFREQ
-   help
- This adds the CPUFreq driver for Samsung EXYNOS5250
- SoC.
-
- If in doubt, say N.
-
 config ARM_EXYNOS5440_CPUFREQ
bool "SAMSUNG EXYNOS5440"
depends on SOC_EXYNOS5440
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 738c8b7..ca5fb2d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -52,9 +52,7 @@ obj-$(CONFIG_ARM_DT_BL_CPUFREQ)   += 
arm_big_little_dt.o
 obj-$(CONFIG_ARCH_DAVINCI_DA850)   += davinci-cpufreq.o
 obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
 obj-$(CONFIG_ARM_EXYNOS_CPUFREQ)   += exynos-cpufreq.o
-obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ)   += exynos4210-cpufreq.o
 obj-$(CONFIG_ARM_EXYNOS4X12_CPUFREQ)   += exynos4x12-cpufreq.o
-obj-$(CONFIG_ARM_EXYNOS5250_CPUFREQ)   += exynos5250-cpufreq.o
 obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ)   += exynos5440-cpufreq.o
 obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
 obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)+= imx6q-cpufreq.o
diff --git a/drivers/cpufreq/exynos4210-cpufreq.c 
b/drivers/cpufreq/exynos4210-cpufreq.c
deleted file mode 100644
index 61a5431..000
--- a/drivers/cpufreq/exynos4210-cpufreq.c
+++ /dev/null
@@ -1,184 +0,0 @@
-/*
- * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
- * http://www.samsung.com
- *
- * EXYNOS4210 - CPU frequency scaling support
- *
- * 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 
-#include 
-#include 
-#include 
-
-#include "exynos-cpufreq.h"
-
-static struct clk *cpu_clk;
-static struct clk *moutcore;
-static struct clk *mout_mpll;
-static struct clk *mout_apll;
-static struct exynos_dvfs_info *cpufreq;
-
-static unsigned int exynos4210_volt_table[] = {
-   125, 115, 105, 975000, 95,
-};
-
-static struct cpufreq_frequency_table exynos4210_freq_table[] = {
-   {0, L0, 1200 * 1000},
-   {0, L1, 1000 * 1000},
-   {0, L2,  800 * 1000},
-   {0, L3,  500 * 1000},
-   {0, L4,  200 * 1000},
-   {0, 0, CPUFREQ_TABLE_END},
-};
-
-static struct apll_freq apll_freq_4210[] = {
-   /*
-* values:
-* freq
-* clock divider for CORE, COREM0, COREM1, PERIPH, ATB, PCLK_DBG, APLL, 
RESERVED
-* clock divider for COPY, HPM, RESERVED
-* PLL M, P, S
-*/
-   APLL_FREQ(1200, 0, 3, 7, 3, 4, 1, 7, 0, 5, 0, 0, 150, 3, 1),
-   APLL_FREQ(1000, 0, 3, 7, 3, 4, 1, 7, 0, 4, 0, 0, 250, 6, 1),
-   APLL_FREQ(800,  0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 200, 6, 1),
-   APLL_FREQ(500,  0, 3, 7, 3, 3, 1, 7, 0, 3, 0, 0, 250, 6, 2),
-   APLL_FREQ(200,  0, 1, 3, 1, 3, 1, 0, 0, 3, 0, 0, 200, 6, 3),
-};
-
-static void exynos4210_set_clkdiv(unsigned int div_index)
-{
-   unsigned int tmp;
-
-   /* Change Divider - CPU0 */
-
-   tmp = apll_freq_4210[div_index].clk_div_cpu0;
-
-   __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS4_CLKDIV_CPU);
-
-   do {
-   tmp = __raw_readl(cpufreq->cmu_regs + EXYNOS4_CLKDIV_STATCPU);
-   } while (tmp & 0x111);
-
-   /* Change Divider - CPU1 */
-
-   tmp = apll_freq_4210[div_index].clk_div_cpu1;
-
-   __raw_writel(tmp, cpufreq->cmu_regs + EXYNOS4_CLKDIV_CPU

[PATCH v6 0/6] cpufreq: use generic cpufreq drivers for exynos platforms

[Thomas Abraham]

Changes since v5:
- Configuration data for cpu clock block is embedded with the code. The cpu 
clock
  logic can later to extended to obtain this data from DT.
- Excluded the support for Exynos4x12 SoC since the work on boost OPP bindings 
is
  still incomplete.
- Included cpufreq support for Exynos5420 SoC.
- Many other minor changes (and so dropped Ack's for some of the patches in v5)

This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
driver.

Thomas Abraham (6):
  clk: samsung: add infrastructure to register cpu clocks
  clk: samsung: register exynos5420 apll/kpll configuration data
  clk: exynos: use cpu-clock provider type to represent arm clock
  ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
  ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
  cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support

 arch/arm/boot/dts/exynos4210-origen.dts |6 +
 arch/arm/boot/dts/exynos4210-trats.dts  |6 +
 arch/arm/boot/dts/exynos4210-universal_c210.dts |6 +
 arch/arm/boot/dts/exynos4210.dtsi   |   27 ++
 arch/arm/boot/dts/exynos5250-arndale.dts|6 +
 arch/arm/boot/dts/exynos5250-cros-common.dtsi   |6 +
 arch/arm/boot/dts/exynos5250-smdk5250.dts   |6 +
 arch/arm/boot/dts/exynos5250.dtsi   |   23 +
 arch/arm/boot/dts/exynos5420-smdk5420.dts   |6 +
 arch/arm/boot/dts/exynos5420.dtsi   |   32 ++
 arch/arm/mach-exynos/exynos.c   |   15 +-
 drivers/clk/samsung/Makefile|2 +-
 drivers/clk/samsung/clk-cpu.c   |  577 +++
 drivers/clk/samsung/clk-exynos4.c   |   25 +-
 drivers/clk/samsung/clk-exynos5250.c|   16 +-
 drivers/clk/samsung/clk-exynos5420.c|   59 ++-
 drivers/clk/samsung/clk.h   |5 +
 drivers/cpufreq/Kconfig.arm |   22 -
 drivers/cpufreq/Makefile|2 -
 drivers/cpufreq/exynos4210-cpufreq.c|  184 
 drivers/cpufreq/exynos5250-cpufreq.c|  210 -
 include/dt-bindings/clock/exynos5250.h  |1 +
 include/dt-bindings/clock/exynos5420.h  |2 +
 23 files changed, 802 insertions(+), 442 deletions(-)
 create mode 100644 drivers/clk/samsung/clk-cpu.c
 delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
 delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c

-- 
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 RFC] mfd: syscon: Decouple syscon interface from syscon devices

[Tomasz Figa]

Currently a syscon entity can be only registered directly through a
platform device that binds to a dedicated driver. However in certain use
cases it is desirable to make a device used with another driver a syscon
interface provider. For example, certain SoCs (e.g. Exynos) contain
system controller blocks which perform various functions such as power
domain control, CPU power management, low power mode control, but in
addition contain certain IP integration glue, such as various signal
masks, coprocessor power control, etc. In such case, there is a need to
have a dedicated driver for such system controller but also share
registers with other drivers. The latter is where the syscon interface
is helpful.

This patch decouples syscon object from syscon driver, so that it can be
registered from any driver in addition to the original "syscon" platform
driver.

Signed-off-by: Tomasz Figa 
---
 drivers/mfd/syscon.c   | 102 +
 include/linux/mfd/syscon.h |   9 
 2 files changed, 75 insertions(+), 36 deletions(-)

diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..11baaae 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -23,30 +24,28 @@
 #include 
 #include 
 
-static struct platform_driver syscon_driver;
+static DEFINE_SPINLOCK(syscon_list_slock);
+static LIST_HEAD(syscon_list);
 
 struct syscon {
struct regmap *regmap;
+   struct device *dev;
+   struct list_head list;
 };
 
-static int syscon_match_node(struct device *dev, void *data)
-{
-   struct device_node *dn = data;
-
-   return (dev->of_node == dn) ? 1 : 0;
-}
-
 struct regmap *syscon_node_to_regmap(struct device_node *np)
 {
-   struct syscon *syscon;
-   struct device *dev;
+   struct syscon *entry, *syscon = ERR_PTR(-EPROBE_DEFER);
+
+   spin_lock(&syscon_list_slock);
 
-   dev = driver_find_device(&syscon_driver.driver, NULL, np,
-syscon_match_node);
-   if (!dev)
-   return ERR_PTR(-EPROBE_DEFER);
+   list_for_each_entry(entry, &syscon_list, list)
+   if (entry->dev->of_node == np) {
+   syscon = entry;
+   break;
+   }
 
-   syscon = dev_get_drvdata(dev);
+   spin_unlock(&syscon_list_slock);
 
return syscon->regmap;
 }
@@ -68,22 +67,19 @@ struct regmap *syscon_regmap_lookup_by_compatible(const 
char *s)
 }
 EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible);
 
-static int syscon_match_pdevname(struct device *dev, void *data)
-{
-   return !strcmp(dev_name(dev), (const char *)data);
-}
-
 struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
 {
-   struct device *dev;
-   struct syscon *syscon;
+   struct syscon *entry, *syscon = ERR_PTR(-EPROBE_DEFER);
+
+   spin_lock(&syscon_list_slock);
 
-   dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s,
-syscon_match_pdevname);
-   if (!dev)
-   return ERR_PTR(-EPROBE_DEFER);
+   list_for_each_entry(entry, &syscon_list, list)
+   if (!strcmp(dev_name(entry->dev), s)) {
+   syscon = entry;
+   break;
+   }
 
-   syscon = dev_get_drvdata(dev);
+   spin_unlock(&syscon_list_slock);
 
return syscon->regmap;
 }
@@ -121,17 +117,49 @@ static struct regmap_config syscon_regmap_config = {
.reg_stride = 4,
 };
 
+static void devm_syscon_unregister(struct device *dev, void *res)
+{
+   struct syscon *syscon = *(struct syscon **)res;
+
+   spin_lock(&syscon_list_slock);
+   list_del(&syscon->list);
+   spin_unlock(&syscon_list_slock);
+}
+
+int devm_syscon_register(struct device *dev, struct regmap *regmap)
+{
+   struct syscon **ptr, *syscon;
+
+   syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
+   if (!syscon)
+   return -ENOMEM;
+
+   syscon->regmap = regmap;
+   syscon->dev = dev;
+
+   ptr = devres_alloc(devm_syscon_unregister, sizeof(*ptr), GFP_KERNEL);
+   if (!ptr)
+   return -ENOMEM;
+
+   spin_lock(&syscon_list_slock);
+   list_add_tail(&syscon->list, &syscon_list);
+   spin_unlock(&syscon_list_slock);
+
+   *ptr = syscon;
+   devres_add(dev, ptr);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(devm_syscon_register);
+
 static int syscon_probe(struct platform_device *pdev)
 {
struct device *dev = &pdev->dev;
struct syscon_platform_data *pdata = dev_get_platdata(dev);
-   struct syscon *syscon;
+   struct regmap *regmap;
struct resource *res;
void __iomem *base;
-
-   syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
-   if (!syscon)
-   return -ENOMEM;
+   int ret;
 
res = platform_get_resource(pdev, IORESOURCE_ME

Re: [PATCH v4 06/11] ARM: EXYNOS: Add support for mapping PMU base address via DT

[Tomasz Figa]

Hi Pankaj,

On 17.06.2014 08:43, Pankaj Dubey wrote:
> Hi Tomasz,
> 
>> 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).
>>
> 
> Thanks for review. 
> 
> Well I adopted same approach in V1 of this patch series. 
> 
> V1: https://lkml.org/lkml/2014/4/2/48
> 
> So, if we do not have issues with that approach, I think we can map PMU
> address
> one time and use it for all machine files including pmu.c. 

The approach itself is fine, but I believe there is no reason to use fdt
there. My recommendation is to follow the method used to map SYSRAMs in
patch "b3205dea8f ARM: EXYNOS: Map SYSRAM through generic DT bindings"
and taking into account patch "b87abf7deb ARM: exynos: move sysram info
to exynos.c", which moves things around source files.

> Also I can see that early_syscon patch [1] is not progressing anymore,
> so in next version of this series better I remove dependency of early syscon
> and usage
> of regmap.

I have another proposal, basically something I already proposed in
review of one of previous versions of this series. I will send a patch
as a reply to this message.

> 
> 1: https://lkml.org/lkml/2014/4/8/239
> 
> Tomasz, It will be good if you can review remaining patches under this
> series, specially patch [2].
> So that, I can update this series after addressing all comments.
> 
> 2: https://lkml.org/lkml/2014/5/10/26
> 

Most of the patches have already received my reviewed-by tag. I'm
generally hesitating to review remaining ones, because the general
architecture will be quite different after changing things mentioned
above. However let me see and try to point issues I can find.

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

[PATCH v6 4/6] ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data

[Thomas Abraham]

From: Thomas Abraham 

For Exynos 4210/5250/5420 based platforms, add CPU nodes, operating points and
cpu clock data for migrating from Exynos specific cpufreq driver to using
generic cpufreq drivers.

Cc: Kukjin Kim 
Signed-off-by: Thomas Abraham 
---
 arch/arm/boot/dts/exynos4210-origen.dts |6 +
 arch/arm/boot/dts/exynos4210-trats.dts  |6 +
 arch/arm/boot/dts/exynos4210-universal_c210.dts |6 +
 arch/arm/boot/dts/exynos4210.dtsi   |   27 +++
 arch/arm/boot/dts/exynos5250-arndale.dts|6 +
 arch/arm/boot/dts/exynos5250-cros-common.dtsi   |6 +
 arch/arm/boot/dts/exynos5250-smdk5250.dts   |6 +
 arch/arm/boot/dts/exynos5250.dtsi   |   23 
 arch/arm/boot/dts/exynos5420-smdk5420.dts   |6 +
 arch/arm/boot/dts/exynos5420.dtsi   |   32 +++
 10 files changed, 124 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210-origen.dts 
b/arch/arm/boot/dts/exynos4210-origen.dts
index f767c42..49a97fc 100644
--- a/arch/arm/boot/dts/exynos4210-origen.dts
+++ b/arch/arm/boot/dts/exynos4210-origen.dts
@@ -33,6 +33,12 @@
bootargs ="root=/dev/ram0 rw ramdisk=8192 initrd=0x4100,8M 
console=ttySAC2,115200 init=/linuxrc";
};
 
+   cpus {
+   cpu@0 {
+   cpu0-supply = <&buck1_reg>;
+   };
+   };
+
regulators {
compatible = "simple-bus";
#address-cells = <1>;
diff --git a/arch/arm/boot/dts/exynos4210-trats.dts 
b/arch/arm/boot/dts/exynos4210-trats.dts
index f516da9..fe32b6a 100644
--- a/arch/arm/boot/dts/exynos4210-trats.dts
+++ b/arch/arm/boot/dts/exynos4210-trats.dts
@@ -30,6 +30,12 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 
rootwait earlyprintk panic=5";
};
 
+   cpus {
+   cpu: cpu@0 {
+   cpu0-supply = <&varm_breg>;
+   };
+   };
+
regulators {
compatible = "simple-bus";
 
diff --git a/arch/arm/boot/dts/exynos4210-universal_c210.dts 
b/arch/arm/boot/dts/exynos4210-universal_c210.dts
index d50eb3a..8ab12d6 100644
--- a/arch/arm/boot/dts/exynos4210-universal_c210.dts
+++ b/arch/arm/boot/dts/exynos4210-universal_c210.dts
@@ -28,6 +28,12 @@
bootargs = "console=ttySAC2,115200N8 root=/dev/mmcblk0p5 rw 
rootwait earlyprintk panic=5 maxcpus=1";
};
 
+   cpus {
+   cpu: cpu@0 {
+   cpu0-supply = <&vdd_arm_reg>;
+   };
+   };
+
sysram@0202 {
smp-sysram@0 {
status = "disabled";
diff --git a/arch/arm/boot/dts/exynos4210.dtsi 
b/arch/arm/boot/dts/exynos4210.dtsi
index ee3001f..c3a73bf 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -31,6 +31,33 @@
pinctrl2 = &pinctrl_2;
};
 
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <0>;
+   clocks = <&clock CLK_ARM_CLK>;
+   clock-names = "cpu";
+
+   operating-points = <
+   120 125
+   100 115
+   80  1075000
+   50  975000
+   40  975000
+   20  95
+   >;
+   };
+
+   cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a9";
+   reg = <1>;
+   };
+   };
+
sysram@0202 {
compatible = "mmio-sram";
reg = <0x0202 0x2>;
diff --git a/arch/arm/boot/dts/exynos5250-arndale.dts 
b/arch/arm/boot/dts/exynos5250-arndale.dts
index d0de1f5..d9b803b 100644
--- a/arch/arm/boot/dts/exynos5250-arndale.dts
+++ b/arch/arm/boot/dts/exynos5250-arndale.dts
@@ -26,6 +26,12 @@
bootargs = "console=ttySAC2,115200";
};
 
+   cpus {
+   cpu@0 {
+   cpu0-supply = <&buck2_reg>;
+   };
+   };
+
rtc@101E {
status = "okay";
};
diff --git a/arch/arm/boot/dts/exynos5250-cros-common.dtsi 
b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
index 89ac90f..34bb31c 100644
--- a/arch/arm/boot/dts/exynos5250-cros-common.dtsi
+++ b/arch/arm/boot/dts/exynos5250-cros-common.dtsi
@@ -19,6 +19,12 @@
chosen {
};
 
+   cpus {
+   cpu@0 {
+   cpu0-supply = <&buck2_reg>;
+   };
+   };
+
pinctrl@1140 {
/*
 * Disab

[PATCH v6 5/6] ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250

[Thomas Abraham]

From: Thomas Abraham 

Remove the platform device instantiation for Exynos4210/5250 cpufreq
driver and add the platform device for generic cpufreq drivers.

Cc: Kukjin Kim 
Signed-off-by: Thomas Abraham 
---
 arch/arm/mach-exynos/exynos.c |   15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index f38cf7c..cfcfe02 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -181,7 +181,20 @@ void __init exynos_cpuidle_init(void)
 
 void __init exynos_cpufreq_init(void)
 {
-   platform_device_register_simple("exynos-cpufreq", -1, NULL, 0);
+   char *dev_name;
+
+   if (of_machine_is_compatible("samsung,exynos5440"))
+   return;
+   if (of_machine_is_compatible("samsung,exynos5420"))
+   dev_name = "arm-bL-cpufreq-dt";
+   else
+   if (of_machine_is_compatible("samsung,exynos4412") ||
+   of_machine_is_compatible("samsung,exynos4212"))
+   dev_name = "exynos-cpufreq";
+   else
+   dev_name = "cpufreq-cpu0";
+
+   platform_device_register_simple(dev_name, -1, NULL, 0);
 }
 
 void __iomem *sysram_base_addr;
-- 
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 v6 3/6] clk: exynos: use cpu-clock provider type to represent arm clock

[Thomas Abraham]

From: Thomas Abraham 

With the addition of the new Samsung specific cpu-clock type, the
arm clock can be represented as a cpu-clock type and the independent
clock blocks that made up the arm clock can be removed.

Cc: Tomasz Figa 
Signed-off-by: Thomas Abraham 
---
 drivers/clk/samsung/clk-exynos4.c  |   25 +
 drivers/clk/samsung/clk-exynos5250.c   |   16 +++-
 drivers/clk/samsung/clk-exynos5420.c   |   31 ++-
 include/dt-bindings/clock/exynos5250.h |1 +
 include/dt-bindings/clock/exynos5420.h |2 ++
 5 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c 
b/drivers/clk/samsung/clk-exynos4.c
index 4f150c9..04cbcb6 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -471,7 +471,8 @@ static struct samsung_mux_clock exynos4210_mux_clks[] 
__initdata = {
MUX(0, "mout_fimd1", group1_p4210, E4210_SRC_LCD1, 0, 4),
MUX(0, "mout_mipi1", group1_p4210, E4210_SRC_LCD1, 12, 4),
MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_CPU, 8, 1),
-   MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1),
+   MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4210, SRC_CPU, 16, 1, 0,
+   CLK_MUX_READ_ONLY),
MUX(CLK_SCLK_VPLL, "sclk_vpll", sclk_vpll_p4210, SRC_TOP0, 8, 1),
MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4210, SRC_CAM, 0, 4),
MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4210, SRC_CAM, 4, 4),
@@ -530,7 +531,8 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] 
__initdata = {
MUX(0, "mout_jpeg", mout_jpeg_p, E4X12_SRC_CAM1, 8, 1),
MUX(CLK_SCLK_MPLL, "sclk_mpll", mout_mpll_p, SRC_DMC, 12, 1),
MUX(CLK_SCLK_VPLL, "sclk_vpll", mout_vpll_p, SRC_TOP0, 8, 1),
-   MUX(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1),
+   MUX_F(CLK_MOUT_CORE, "mout_core", mout_core_p4x12, SRC_CPU, 16, 1, 0,
+   CLK_MUX_READ_ONLY),
MUX(CLK_MOUT_FIMC0, "mout_fimc0", group1_p4x12, SRC_CAM, 0, 4),
MUX(CLK_MOUT_FIMC1, "mout_fimc1", group1_p4x12, SRC_CAM, 4, 4),
MUX(CLK_MOUT_FIMC2, "mout_fimc2", group1_p4x12, SRC_CAM, 8, 4),
@@ -572,8 +574,10 @@ static struct samsung_mux_clock exynos4x12_mux_clks[] 
__initdata = {
 
 /* list of divider clocks supported in all exynos4 soc's */
 static struct samsung_div_clock exynos4_div_clks[] __initdata = {
-   DIV(0, "div_core", "mout_core", DIV_CPU0, 0, 3),
-   DIV(0, "div_core2", "div_core", DIV_CPU0, 28, 3),
+   DIV_F(0, "div_core", "mout_core", DIV_CPU0, 0, 3,
+   CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+   DIV_F(0, "div_core2", "div_core", DIV_CPU0, 28, 3,
+   CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
DIV(0, "div_fimc0", "mout_fimc0", DIV_CAM, 0, 4),
DIV(0, "div_fimc1", "mout_fimc1", DIV_CAM, 4, 4),
DIV(0, "div_fimc2", "mout_fimc2", DIV_CAM, 8, 4),
@@ -619,8 +623,10 @@ static struct samsung_div_clock exynos4_div_clks[] 
__initdata = {
DIV(0, "div_spi_pre2", "div_spi2", DIV_PERIL2, 8, 8),
DIV(0, "div_audio1", "mout_audio1", DIV_PERIL4, 0, 4),
DIV(0, "div_audio2", "mout_audio2", DIV_PERIL4, 16, 4),
-   DIV(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3),
-   DIV(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3),
+   DIV_F(CLK_ARM_CLK, "arm_clk", "div_core2", DIV_CPU0, 28, 3,
+   CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
+   DIV_F(CLK_SCLK_APLL, "sclk_apll", "mout_apll", DIV_CPU0, 24, 3,
+   CLK_GET_RATE_NOCACHE, CLK_DIVIDER_READ_ONLY),
DIV_F(0, "div_mipi_pre0", "div_mipi0", DIV_LCD0, 20, 4,
CLK_SET_RATE_PARENT, 0),
DIV_F(0, "div_mmc_pre0", "div_mmc0", DIV_FSYS1, 8, 8,
@@ -1005,7 +1011,6 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] 
__initdata = {
 
 static struct samsung_clock_alias exynos4_aliases[] __initdata = {
ALIAS(CLK_MOUT_CORE, NULL, "moutcore"),
-   ALIAS(CLK_ARM_CLK, NULL, "armclk"),
ALIAS(CLK_SCLK_APLL, NULL, "mout_apll"),
 };
 
@@ -1244,6 +1249,8 @@ static void __init exynos4_clk_init(struct device_node 
*np,
ARRAY_SIZE(exynos4210_gate_clks));
samsung_clk_register_alias(ctx, exynos4210_aliases,
ARRAY_SIZE(exynos4210_aliases));
+   exynos_register_cpu_clock(ctx, 0, CLK_ARM_CLK, "armclk",
+   mout_core_p4210[0], mout_core_p4210[1], np);
} else {
samsung_clk_register_mux(ctx, exynos4x12_mux_clks,
ARRAY_SIZE(exynos4x12_mux_clks));
@@ -1253,6 +1260,8 @@ static void __init exynos4_clk_init(struct device_node 
*np,
ARRAY_SIZE(exynos4x12_gate_clks));
samsung_clk_register_alias(ctx, exynos4x12_aliases,
ARRAY_SIZE(exynos4x12_ali

[PATCH v6 2/6] clk: samsung: register exynos5420 apll/kpll configuration data

[Thomas Abraham]

From: Thomas Abraham 

Register the PLL configuration data for APLL and KPLL on Exynos5420. This
configuration data table specifies PLL coefficients for supported PLL
clock speeds when a 24MHz clock is supplied as the input clock source
for these PLLs.

Cc: Tomasz Figa 
Signed-off-by: Thomas Abraham 
---
 drivers/clk/samsung/clk-exynos5420.c |   28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/clk/samsung/clk-exynos5420.c 
b/drivers/clk/samsung/clk-exynos5420.c
index 9d7d7ee..51cff4a 100644
--- a/drivers/clk/samsung/clk-exynos5420.c
+++ b/drivers/clk/samsung/clk-exynos5420.c
@@ -1142,6 +1142,28 @@ static struct samsung_gate_clock exynos5x_gate_clks[] 
__initdata = {
GATE(CLK_G3D, "g3d", "mout_user_aclk_g3d", GATE_IP_G3D, 9, 0, 0),
 };
 
+static const struct samsung_pll_rate_table exynos5420_pll2550x_24mhz_tbl[] = {
+   PLL_35XX_RATE(20, 250, 3, 0),
+   PLL_35XX_RATE(19, 475, 6, 0),
+   PLL_35XX_RATE(18, 225, 3, 0),
+   PLL_35XX_RATE(17, 425, 6, 0),
+   PLL_35XX_RATE(16, 200, 3, 0),
+   PLL_35XX_RATE(15, 250, 4, 0),
+   PLL_35XX_RATE(14, 175, 3, 0),
+   PLL_35XX_RATE(13, 325, 6, 0),
+   PLL_35XX_RATE(12, 200, 2, 1),
+   PLL_35XX_RATE(11, 275, 3, 1),
+   PLL_35XX_RATE(10, 250, 3, 1),
+   PLL_35XX_RATE(9,  150, 2, 1),
+   PLL_35XX_RATE(8,  200, 3, 1),
+   PLL_35XX_RATE(7,  175, 3, 1),
+   PLL_35XX_RATE(6,  200, 2, 2),
+   PLL_35XX_RATE(5,  250, 3, 2),
+   PLL_35XX_RATE(4,  200, 3, 2),
+   PLL_35XX_RATE(3,  200, 2, 3),
+   PLL_35XX_RATE(2,  200, 3, 3),
+};
+
 static struct samsung_pll_clock exynos5x_plls[nr_plls] __initdata = {
[apll] = PLL(pll_2550, CLK_FOUT_APLL, "fout_apll", "fin_pll", APLL_LOCK,
APLL_CON0, NULL),
@@ -1195,6 +1217,12 @@ static void __init exynos5x_clk_init(struct device_node 
*np,
samsung_clk_of_register_fixed_ext(ctx, exynos5x_fixed_rate_ext_clks,
ARRAY_SIZE(exynos5x_fixed_rate_ext_clks),
ext_clk_match);
+
+   if (_get_rate("fin_pll") == 24 * MHZ) {
+   exynos5x_plls[apll].rate_table = exynos5420_pll2550x_24mhz_tbl;
+   exynos5x_plls[kpll].rate_table = exynos5420_pll2550x_24mhz_tbl;
+   }
+
samsung_clk_register_pll(ctx, exynos5x_plls, ARRAY_SIZE(exynos5x_plls),
reg_base);
samsung_clk_register_fixed_rate(ctx, exynos5x_fixed_rate_clks,
-- 
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 v6 1/6] clk: samsung: add infrastructure to register cpu clocks

[Thomas Abraham]

From: Thomas Abraham 

The CPU clock provider supplies the clock to the CPU clock domain. The
composition and organization of the CPU clock provider could vary among
Exynos SoCs. A CPU clock provider can be composed of clock mux, dividers
and gates. This patch defines a new clock type for CPU clock provider and
adds infrastructure to register the CPU clock providers for Samsung
platforms.

Cc: Tomasz Figa 
Signed-off-by: Thomas Abraham 
---
 drivers/clk/samsung/Makefile  |2 +-
 drivers/clk/samsung/clk-cpu.c |  577 +
 drivers/clk/samsung/clk.h |5 +
 3 files changed, 583 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/samsung/clk-cpu.c

diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 69e8177..f4edd31 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -2,7 +2,7 @@
 # Samsung Clock specific Makefile
 #
 
-obj-$(CONFIG_COMMON_CLK)   += clk.o clk-pll.o
+obj-$(CONFIG_COMMON_CLK)   += clk.o clk-pll.o clk-cpu.o
 obj-$(CONFIG_SOC_EXYNOS3250)   += clk-exynos3250.o
 obj-$(CONFIG_ARCH_EXYNOS4) += clk-exynos4.o
 obj-$(CONFIG_SOC_EXYNOS5250)   += clk-exynos5250.o
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
new file mode 100644
index 000..c40f7b5
--- /dev/null
+++ b/drivers/clk/samsung/clk-cpu.c
@@ -0,0 +1,577 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ * Author: Thomas Abraham 
+ *
+ * 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.
+ *
+ * This file contains the utility functions to register the CPU clocks
+ * for Samsung platforms.
+*/
+
+#include 
+#include "clk.h"
+
+#define E4210_SRC_CPU  0x0
+#define E4210_STAT_CPU 0x200
+#define E4210_DIV_CPU0 0x300
+#define E4210_DIV_CPU1 0x304
+#define E4210_DIV_STAT_CPU00x400
+#define E4210_DIV_STAT_CPU10x404
+
+#define MAX_DIV8
+#define DIV_MASK   7
+#define DIV_MASK_ALL   0x
+#define MUX_MASK   7
+
+#define E4210_DIV0_RATIO0_MASK 0x7
+#define E4210_DIV1_HPM_MASK((0x7 << 4) | (0x7 << 0))
+#define E4210_MUX_HPM_MASK (1 << 20)
+#define E4210_DIV0_ATB_SHIFT   16
+#define E4210_DIV0_ATB_MASK(DIV_MASK << E4210_DIV0_ATB_SHIFT)
+
+#define E4210_CPU_DIV0(apll, pclk_dbg, atb, periph, corem1, corem0)\
+   (((apll) << 24) | ((pclk_dbg) << 20) | ((atb) << 16) |  \
+   ((periph) << 12) | ((corem1) << 8) | ((corem0) <<  4))
+#define E4210_CPU_DIV1(hpm, copy)  \
+   (((hpm) << 4) | ((copy) << 0))
+
+#define E5250_CPU_DIV0(apll, pclk_dbg, atb, periph, acp, cpud) \
+   (((apll << 24) | (pclk_dbg << 20) | (atb << 16) |   \
+(periph << 12) | (acp << 8) | (cpud << 4)))
+#define E5250_CPU_DIV1(hpm, copy)  \
+   (((hpm) << 4) | (copy))
+
+#define E5420_EGL_DIV0(apll, pclk_dbg, atb, cpud)  \
+   (((apll << 24) | (pclk_dbg << 20) | (atb << 16) |   \
+(cpud << 4)))
+#define E5420_KFC_DIV(kpll, pclk, aclk)
\
+   (((kpll << 24) | (pclk << 20) | (aclk << 4)))
+
+enum cpuclk_type {
+   EXYNOS4210,
+   EXYNOS5250,
+   EXYNOS5420,
+};
+
+/**
+ * struct exynos4210_cpuclk_data: config data to setup cpu clocks.
+ * @prate: frequency of the primary parent clock (in KHz).
+ * @div0: value to be programmed in the div_cpu0 register.
+ * @div1: value to be programmed in the div_cpu1 register.
+ *
+ * This structure holds the divider configuration data for dividers in the CPU
+ * clock domain. The parent frequency at which these divider values are valid 
is
+ * specified in @prate. The @prate is the frequency of the primary parent 
clock.
+ * For CPU clock domains that do not have a DIV1 register, the @div1 member
+ * is optional.
+ */
+struct exynos4210_cpuclk_data {
+   unsigned long   prate;
+   unsigned intdiv0;
+   unsigned intdiv1;
+};
+
+/**
+ * struct exynos_cpuclk: information about clock supplied to a CPU core.
+ * @hw:handle between CCF and CPU clock.
+ * @alt_parent: alternate parent clock to use when switching the speed
+ * of the primary parent clock.
+ * @ctrl_base: base address of the clock controller.
+ * @offset: offset from the ctrl_base address where the CPU clock div/mux
+ * registers can be accessed.
+ * @lock: cpu clock domain register access lock.
+ * @type: type of the CPU clock.
+ * @data: optional data which the actual instantiation of this clock
+ * can use.
+ * @clk_nb: clock notifier registered for changes in clock speed of the
+ * primary parent clock.
+ * @pre_rate_cb: callback function to handle PRE_RATE_CHANGE notification
+ *   

[PATCH v6 0/6] cpufreq: use generic cpufreq drivers for exynos platforms

[Thomas Abraham]

Changes since v5:
- Configuration data for cpu clock block is embedded with the code. The cpu 
clock
  logic can later to extended to obtain this data from DT.
- Excluded the support for Exynos4x12 SoC since the work on boost OPP bindings 
is
  still incomplete.
- Included cpufreq support for Exynos5420 SoC.
- Many other minor changes (and so dropped Ack's for some of the patches in v5)

This patch series removes the use of Exynos4210 and Exynos5250 specific cpufreq
drivers and enables the use of cpufreq-cpu0 driver for these platforms. This
series also enabled cpufreq support for Exynos5420 using arm_big_little cpufreq
driver.

Thomas Abraham (6):
  clk: samsung: add infrastructure to register cpu clocks
  clk: samsung: register exynos5420 apll/kpll configuration data
  clk: exynos: use cpu-clock provider type to represent arm clock
  ARM: dts: Exynos: add cpu nodes, opp and cpu clock configuration data
  ARM: Exynos: switch to using generic cpufreq driver for exynos4210/5250
  cpufreq: exynos: remove exynos4210/5250 specific cpufreq driver support

 arch/arm/boot/dts/exynos4210-origen.dts |6 +
 arch/arm/boot/dts/exynos4210-trats.dts  |6 +
 arch/arm/boot/dts/exynos4210-universal_c210.dts |6 +
 arch/arm/boot/dts/exynos4210.dtsi   |   27 ++
 arch/arm/boot/dts/exynos5250-arndale.dts|6 +
 arch/arm/boot/dts/exynos5250-cros-common.dtsi   |6 +
 arch/arm/boot/dts/exynos5250-smdk5250.dts   |6 +
 arch/arm/boot/dts/exynos5250.dtsi   |   23 +
 arch/arm/boot/dts/exynos5420-smdk5420.dts   |6 +
 arch/arm/boot/dts/exynos5420.dtsi   |   32 ++
 arch/arm/mach-exynos/exynos.c   |   15 +-
 drivers/clk/samsung/Makefile|2 +-
 drivers/clk/samsung/clk-cpu.c   |  577 +++
 drivers/clk/samsung/clk-exynos4.c   |   25 +-
 drivers/clk/samsung/clk-exynos5250.c|   16 +-
 drivers/clk/samsung/clk-exynos5420.c|   59 ++-
 drivers/clk/samsung/clk.h   |5 +
 drivers/cpufreq/Kconfig.arm |   22 -
 drivers/cpufreq/Makefile|2 -
 drivers/cpufreq/exynos4210-cpufreq.c|  184 
 drivers/cpufreq/exynos5250-cpufreq.c|  210 -
 include/dt-bindings/clock/exynos5250.h  |1 +
 include/dt-bindings/clock/exynos5420.h  |2 +
 23 files changed, 802 insertions(+), 442 deletions(-)
 create mode 100644 drivers/clk/samsung/clk-cpu.c
 delete mode 100644 drivers/cpufreq/exynos4210-cpufreq.c
 delete mode 100644 drivers/cpufreq/exynos5250-cpufreq.c

-- 
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 v2] devicetree: Add generic IOMMU device tree bindings

[Stuart Yoder]

> -Original Message-
> From: Sethi Varun-B16395
> Sent: Tuesday, June 17, 2014 6:22 AM
> To: Will Deacon
> Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung-
> s...@vger.kernel.org; Arnd Bergmann; Pawel Moll; Ian Campbell; Grant
> Grundler; Stephen Warren; Yoder Stuart-B08248; Rob Herring; linux-
> ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Thierry Reding; Kumar
> Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm-
> ker...@lists.infradead.org
> Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree
> bindings
> 
> 
> 
> > -Original Message-
> > From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> > boun...@lists.linux-foundation.org] On Behalf Of Will Deacon
> > Sent: Tuesday, June 17, 2014 4:13 PM
> > To: Sethi Varun-B16395
> > Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung-
> > s...@vger.kernel.org; Arnd Bergmann; Pawel Moll; Ian Campbell; Grant
> > Grundler; Stephen Warren; Yoder Stuart-B08248; Rob Herring; linux-
> > ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Thierry Reding;
> Kumar
> > Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-
> arm-
> > ker...@lists.infradead.org
> > Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree
> > bindings
> >
> > On Tue, Jun 17, 2014 at 11:26:48AM +0100, Varun Sethi wrote:
> > > > The way we generally thought it would work was something like
> > > > this:
> > > >-u-boot/bootloader makes any static streamID allocation if
> needed,
> > > > sets a default streamID  (e.g. 0x0) in device and expresses
> > > > that in the device tree
> > > >-device tree would express relationship between devices
> > > > (including bus controllers) and the SMMU through mmu-masters
> > > > property
> > > >-u-boot would express the range of unused (or used) streamIDs
> via
> > > > a new
> > > > device tree property so the kernel SMMU driver knows what
> > > > streamIDs are
> > > > free
> > > >-in the SMMU driver a different vendor specific 'add_device'
> > callback
> > > > could be used to handle our special cases where we need to
> > set/change
> > > > the stream ID for devices added to a domain
> > >
> > > Another possibility, could be to program the stream Id in the device
> > > registers (reference for the stream ID register can be obtained from
> > > the device tree) during device attach. This could be relevant in case
> > > of VFIO, when we are assigning multiple devices to a single VM. All
> > > the devices can share the same stream ID.
> >
> > I think for simple masters (i.e. those that have all their StreamIDs
> > under control of one driver), then setting something during attach (or
> > add?) based on the DT could work pretty well. The other case is when we
> > have masters behind a bridge (such as a PCI RC). In this case, it might
> > actually be better to ask the bridge for the IDs and let it sort out
> the
> > allocation itself. That would also move the RequesterID -> StreamID
> > mapping out of the SMMU code.
> >
> > What do you think?
> The PCI bus iommu group creation code would be part of the SMMU driver
> (it is handled by other IOMMU drivers as well). My understanding is that
> there would be one is to one correspondence between the requestor ID and
> the iommu group. May be, we can have an API provided by the PCI bridge
> (architecture specific) for setting the stream ID.

I think Will is suggesting something along those lines-- I think it's a
question of where the streamID allocation happens.  You could
either do something like the following in the SMMU driver when attaching
a PCI device:

id = alloc_stream_id(...);
pci_set_streamid(pci-dev, id);

or

id = pci_get_streamid(pci-dev);

...i.e the PCI RC could allocate (from some TBD
allocator) and set the stream ID itself.

Not sure how big a deal it is to extend PCI RC interfaces for 
something like that.

Stuart
--
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] devicetree: Add generic IOMMU device tree bindings

[Stuart Yoder]

> -Original Message-
> From: Sethi Varun-B16395
> Sent: Tuesday, June 17, 2014 5:27 AM
> To: Yoder Stuart-B08248; Will Deacon
> Cc: Thierry Reding; Mark Rutland; devicet...@vger.kernel.org; linux-
> samsung-...@vger.kernel.org; Pawel Moll; Arnd Bergmann; Ian Campbell;
> Grant Grundler; Stephen Warren; linux-ker...@vger.kernel.org; Marc
> Zyngier; Linux IOMMU; Rob Herring; Kumar Gala; linux-
> te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm-
> ker...@lists.infradead.org
> Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree
> bindings
> 
> 
> 
> > -Original Message-
> > From: Yoder Stuart-B08248
> > Sent: Tuesday, June 17, 2014 12:24 AM
> > To: Will Deacon
> > Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland;
> > devicet...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Pawel
> > Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren;
> linux-
> > ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar
> > Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-
> arm-
> > ker...@lists.infradead.org
> > Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree
> > bindings
> >
> >
> >
> > > -Original Message-
> > > From: Will Deacon [mailto:will.dea...@arm.com]
> > > Sent: Monday, June 16, 2014 12:04 PM
> > > To: Yoder Stuart-B08248
> > > Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland;
> > > devicet...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Pawel
> > > Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren;
> > > linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob
> Herring;
> > > Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin;
> > > linux-arm- ker...@lists.infradead.org
> > > Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree
> > > bindings
> > >
> > > Hi Stuart,
> > >
> > > On Mon, Jun 16, 2014 at 05:56:32PM +0100, Stuart Yoder wrote:
> > > > > Do you have use-cases where you really need to change these
> > > > > mappings dynamically?
> > > >
> > > > Yes.  In the case of a PCI bus-- you may not know in advance how
> many
> > > > PCI devices there are until you probe the bus.   We have another
> FSL
> > > > proprietary bus we call the "fsl-mc" bus that is similar.
> > >
> > > For that case, though, you could still describe an algorithmic
> > > transformation from RequesterID to StreamID which corresponds to a
> > > fixed mapping.
> > >
> > > > Another thing to consider-- starting with SMMUv2, as you know,
> there
> > > > is a new distributed architecture with multiple TBUs and a
> > > > centralized TCU that walks the SMMU page tables.  So instead of
> > > > sprinkling multiple SMMUs all over an SoC you now have the option a
> > > > 1 central TCU and
> > > sprinkling
> > > > multiple TBUs around.   However, this means that the stream ID
> > > namespace
> > > > is now global and can be pretty limited.  In the SMMU
> implementation
> > > > we have there are only 64 stream ID total for our Soc.  But we have
> > > > many
> > > more
> > > > masters than that.
> > > >
> > > > So we look at stream IDs as really corresponding to an 'isolation
> > > context'
> > > > and not to a bus master.  An isolation context is the domain you
> are
> > > > trying to isolate with the SMMU.  Devices that all belong to the
> > > > same 'isolation context' can share the same stream ID, since they
> > > > share the same domain and page tables.
> > >
> > > Ok, this is more compelling.
> > >
> > > > So, perhaps by default some/most SMMU masters may have a default
> > > > stream
> > > ID
> > > > of 0x0 that is used by the host...and that could be represented
> > > > statically in the device tree.
> > > >
> > > > But, we absolutely will need to dynamically set new stream IDs into
> > > > masters when a new IOMMU 'domain' is created and devices
> > > > are added to it.   All the devices in a domain will share
> > > > the same stream ID.
> > > >
> > > > So whatever we do, let's please have an architecture flexible
> enough
> > > > to allow for this.
> > >
> > > What is the software interface to the logic that assigns the
> StreamIDs?
> > > Is
> > > it part of the SMMU, or a separate device (or set of devices)?
> >
> > For us at the hardware level there are a few different ways that the
> > streamIDs can be set.  It is not part of the SMMU.  In the cases where
> > there is simply
> > 1 device to 1 streamID (e.g. USB controller) there is an SoC register
> > where
> > you just program the stream ID.   In the case of PCI, our PCI
> controller
> > has a RequesterID-to-streamID table that you set via some PCI
> controller
> > registers.
> >
> > The way we generally thought it would work was something like
> > this:
> >-u-boot/bootloader makes any static streamID allocation if needed,
> > sets a default streamID  (e.g. 0x0) in device and expresses
> > that in the device tree
> >-device tree would express relationship between devices
> > (including

Re: [PATCH v2 07/10] regulator: Add driver for Maxim 77802 PMIC regulators

[Mark Brown]

On Tue, Jun 17, 2014 at 12:49:56PM +0200, Javier Martinez Canillas wrote:
> On 06/16/2014 09:25 PM, Mark Brown wrote:

> >> +  config.dev = &pdev->dev;

> > Are you sure this shouldn't be the MFD?

> I just looked at regulator_register() and saw that it does rdev->dev.parent =
> dev, so yes this has to be the MFD.

Do the regulators manage to get their supplies?

> So, for now I thought it made sense to set the operating mode to normal on
> probe() but I'll change it to read from the hardware if that is better.

Yes, otherwise if the device is configured otherwise then when we change
the configuration we may break something.

> I guess I should check in the datasheet if a sane default operating mode for
> LDOs is expected when the chip is reseted or if this is left undefined and 
> also
> if the bootloader already set this.

You can't do anything based on the particular bootloader you're using in
your current system, this has to work in other systems.


signature.asc
Description: Digital signature

Re: Boot warnings on exynos5420 based boards

[Srivatsa S. Bhat]

On 06/17/2014 05:25 PM, Sachin Kamat wrote:
> Hi Srivatsa,
> 
> On Tue, Jun 17, 2014 at 3:24 PM, Srivatsa S. Bhat
>  wrote:
>> On 06/17/2014 03:03 PM, Sachin Kamat wrote:
>>
 Below is an updated patch, please let me know how it goes. (You'll have to
 revert c47a9d7cca first, and then 56e692182, before trying this patch).
>>>
>>> I am unable to apply your below patch on top of the above 2 reverts.
>>> Applying: CPU hotplug, smp: Execute any pending IPI callbacks before CPU 
>>> offline
>>> fatal: corrupt patch at line 106
>>> Patch failed at 0001 CPU hotplug, smp: Execute any pending IPI
>>> callbacks before CPU offline
>>>
>>> Even with 'patch' I get the below failures:
>>> patching file kernel/smp.c
>>> Hunk #2 FAILED at 53.
>>> Hunk #3 FAILED at 179.
>>> 2 out of 3 hunks FAILED -- saving rejects to file kernel/smp.c.rej
>>>
>>
>> Hmm, weird. My mailer must have screwed it up.
>>
>> Let's try again:
>>
>> [In case this also doesn't work for you, please use this git tree in which
>>  I have reverted the 2 old commits and added this updated patch.
>>
>>  git://github.com/srivatsabhat/linux.git ipi-offline-fix-v3
>> ]
> 
> Unfortunately the attached patch did not apply either. Nevertheless, I
> applied the
> patch from your above mentioned tree. With that patch I do not see the 
> warnings
> that I mentioned in my first mail. Thanks for fixing it.
> 

Sure, thanks for reporting the bug and testing the updated patch!
By the way, I think there is some problem in the workflow that you use to
copy-paste/apply the patch. I tried applying both patches (that I sent in
2 different mails) and both applied properly without any problems.

Regards,
Srivatsa S. Bhat

--
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] devicetree: Add generic IOMMU device tree bindings

[Will Deacon]

On Tue, Jun 17, 2014 at 12:58:30PM +0100, Thierry Reding wrote:
> On Mon, Jun 16, 2014 at 01:57:04PM +0100, Will Deacon wrote:
> > On Wed, Jun 04, 2014 at 10:12:38PM +0100, Thierry Reding wrote:
> > > It can easily be argued that if the algorithm used to remap the ID
> > > varies, the compatibility of the device changes. Therefore I would
> > > expect any variant of the GICv3 that deviates from the "standard"
> > > mapping (if there is such a thing) to have its own compatible string.
> > 
> > There is no standard mapping; it's a property defined at system integration
> > time. I fully expect different SoCs to do different things here.
> 
> My point was that the mapping itself seems to be fundamental enough to
> make devices with different mappings "incompatible". Therefore I think
> this could probably be handled by using different compatible values,
> something along the lines of this:
> 
>   compatible = "vendor,soc-gicv3", "arm,gicv3";
> 
> Then the mapping can be described in code, which should be a whole lot
> easier and more flexible than a more or less generic notation in device
> tree.

I don't think that scales well beyond a handful of unique mappings, and I
really anticipate everybody doing something different based on their
integration constraints.

You'd very quickly end up with sets of tables for each SoC, describing the
topology and associated IDs in the kernel source, which feels like a giant
step backwards from where we are today with device tree.

If, for example, the GIC architecture prescribed a fixed set of Device IDs
and an algorithm for converting from Stream IDs then your approach may have
some merits, but that's not where we are today (and I also don't think it's
practical to try and enforce such system-wide properties into an interrupt
controller architecture).

Will
--
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] clocksource: exynos_mct: Fix ftrace

[Daniel Lezcano]

On 06/04/2014 07:30 PM, Doug Anderson wrote:

In (93bfb76 clocksource: exynos_mct: register sched_clock callback) we
supported using the MCT as a scheduler clock.  We properly marked
exynos4_read_sched_clock() as notrace.  However, we then went and
called another function that _wasn't_ notrace.  That means if you do:

   cd /sys/kernel/debug/tracing/
   echo function_graph > current_tracer

You'll get a crash.

Fix this (but still let other readers of the MCT be trace-enabled) by
adding an extra function.  It's important to keep other users of MCT
traceable because the MCT is actually quite slow.


Thanks for the explanation in the other email.

I think the last sentence is a bit confusing because you are implicitly 
saying you need these traces to investigate why the timer is slow which 
is referring to something not related to this fix.



Signed-off-by: Doug Anderson 
---
  drivers/clocksource/exynos_mct.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 8d64200..ba3a683 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -165,7 +165,7 @@ static void exynos4_mct_frc_start(u32 hi, u32 lo)
exynos4_mct_write(reg, EXYNOS4_MCT_G_TCON);
  }

-static cycle_t exynos4_frc_read(struct clocksource *cs)
+static inline cycle_t notrace _exynos4_frc_read(void)


Why inline ?


  {
unsigned int lo, hi;
u32 hi2 = __raw_readl(reg_base + EXYNOS4_MCT_G_CNT_U);
@@ -179,6 +179,11 @@ static cycle_t exynos4_frc_read(struct clocksource *cs)
return ((cycle_t)hi << 32) | lo;
  }

+static cycle_t exynos4_frc_read(struct clocksource *cs)
+{
+   return _exynos4_frc_read();
+}
+
  static void exynos4_frc_resume(struct clocksource *cs)
  {
exynos4_mct_frc_start(0, 0);
@@ -195,7 +200,7 @@ struct clocksource mct_frc = {

  static u64 notrace exynos4_read_sched_clock(void)
  {
-   return exynos4_frc_read(&mct_frc);
+   return _exynos4_frc_read();
  }

  static void __init exynos4_clocksource_init(void)




--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
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] devicetree: Add generic IOMMU device tree bindings

[Thierry Reding]

On Mon, Jun 16, 2014 at 01:57:04PM +0100, Will Deacon wrote:
> On Wed, Jun 04, 2014 at 10:12:38PM +0100, Thierry Reding wrote:
> > On Fri, May 30, 2014 at 12:27:28PM +0100, Dave Martin wrote:
> > > On Fri, May 30, 2014 at 08:30:08AM +0100, Thierry Reding wrote:
> > [...]
> > > > Arnd, can you take another look at this binding and see if there's
> > > > anything else missing? If not I'll go through the document again and
> > > > update all #address-cells/#size-cells references with #iommu-cells as
> > > > appropriate and submit v3.
> > > 
> > > How do you envisage propagation of the master ID bits downstream of the
> > > IOMMU would be described?
> > > 
> > > We will definitely need a way to describe this for GICv3.  How those
> > > values are propagated is likely to vary between related SoCs and doesn't
> > > feel like it should be baked into a driver, especially for the ARM SMMU
> > > which may get reused in radically different SoC families from different
> > > vendors.
> > 
> > Well, we've had cases like these in the past (power sequences come to
> > mind). Some concepts are just too difficult or unwieldy to be put into
> > device tree. I think that this is one of them.
> > 
> > > The most likely types of remapping are the adding of a base offset or
> > > some extra bits to the ID -- because not all MSIs to the GIC will
> > > necessarily pass through the IOMMU.  It's also possible that we might
> > > see ID squashing or folding in some systems.
> > 
> > It can easily be argued that if the algorithm used to remap the ID
> > varies, the compatibility of the device changes. Therefore I would
> > expect any variant of the GICv3 that deviates from the "standard"
> > mapping (if there is such a thing) to have its own compatible string.
> 
> There is no standard mapping; it's a property defined at system integration
> time. I fully expect different SoCs to do different things here.

My point was that the mapping itself seems to be fundamental enough to
make devices with different mappings "incompatible". Therefore I think
this could probably be handled by using different compatible values,
something along the lines of this:

compatible = "vendor,soc-gicv3", "arm,gicv3";

Then the mapping can be described in code, which should be a whole lot
easier and more flexible than a more or less generic notation in device
tree.

Thierry


pgpWIyVt9WhEY.pgp
Description: PGP signature

Re: Boot warnings on exynos5420 based boards

[Sachin Kamat]

Hi Srivatsa,

On Tue, Jun 17, 2014 at 3:24 PM, Srivatsa S. Bhat
 wrote:
> On 06/17/2014 03:03 PM, Sachin Kamat wrote:
>
>>> Below is an updated patch, please let me know how it goes. (You'll have to
>>> revert c47a9d7cca first, and then 56e692182, before trying this patch).
>>
>> I am unable to apply your below patch on top of the above 2 reverts.
>> Applying: CPU hotplug, smp: Execute any pending IPI callbacks before CPU 
>> offline
>> fatal: corrupt patch at line 106
>> Patch failed at 0001 CPU hotplug, smp: Execute any pending IPI
>> callbacks before CPU offline
>>
>> Even with 'patch' I get the below failures:
>> patching file kernel/smp.c
>> Hunk #2 FAILED at 53.
>> Hunk #3 FAILED at 179.
>> 2 out of 3 hunks FAILED -- saving rejects to file kernel/smp.c.rej
>>
>
> Hmm, weird. My mailer must have screwed it up.
>
> Let's try again:
>
> [In case this also doesn't work for you, please use this git tree in which
>  I have reverted the 2 old commits and added this updated patch.
>
>  git://github.com/srivatsabhat/linux.git ipi-offline-fix-v3
> ]

Unfortunately the attached patch did not apply either. Nevertheless, I
applied the
patch from your above mentioned tree. With that patch I do not see the warnings
that I mentioned in my first mail. Thanks for fixing it.

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: [PATCH v2] devicetree: Add generic IOMMU device tree bindings

[Varun Sethi]

> -Original Message-
> From: iommu-boun...@lists.linux-foundation.org [mailto:iommu-
> boun...@lists.linux-foundation.org] On Behalf Of Will Deacon
> Sent: Tuesday, June 17, 2014 4:13 PM
> To: Sethi Varun-B16395
> Cc: Mark Rutland; devicet...@vger.kernel.org; linux-samsung-
> s...@vger.kernel.org; Arnd Bergmann; Pawel Moll; Ian Campbell; Grant
> Grundler; Stephen Warren; Yoder Stuart-B08248; Rob Herring; linux-
> ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Thierry Reding; Kumar
> Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm-
> ker...@lists.infradead.org
> Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree
> bindings
> 
> On Tue, Jun 17, 2014 at 11:26:48AM +0100, Varun Sethi wrote:
> > > The way we generally thought it would work was something like
> > > this:
> > >-u-boot/bootloader makes any static streamID allocation if needed,
> > > sets a default streamID  (e.g. 0x0) in device and expresses
> > > that in the device tree
> > >-device tree would express relationship between devices
> > > (including bus controllers) and the SMMU through mmu-masters
> > > property
> > >-u-boot would express the range of unused (or used) streamIDs via
> > > a new
> > > device tree property so the kernel SMMU driver knows what
> > > streamIDs are
> > > free
> > >-in the SMMU driver a different vendor specific 'add_device'
> callback
> > > could be used to handle our special cases where we need to
> set/change
> > > the stream ID for devices added to a domain
> >
> > Another possibility, could be to program the stream Id in the device
> > registers (reference for the stream ID register can be obtained from
> > the device tree) during device attach. This could be relevant in case
> > of VFIO, when we are assigning multiple devices to a single VM. All
> > the devices can share the same stream ID.
> 
> I think for simple masters (i.e. those that have all their StreamIDs
> under control of one driver), then setting something during attach (or
> add?) based on the DT could work pretty well. The other case is when we
> have masters behind a bridge (such as a PCI RC). In this case, it might
> actually be better to ask the bridge for the IDs and let it sort out the
> allocation itself. That would also move the RequesterID -> StreamID
> mapping out of the SMMU code.
> 
> What do you think?
The PCI bus iommu group creation code would be part of the SMMU driver (it is 
handled by other IOMMU drivers as well). My understanding is that there would 
be one is to one correspondence between the requestor ID and the iommu group. 
May be, we can have an API provided by the PCI bridge (architecture specific) 
for setting the stream ID.

-Varun
--
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 06/10] mfd: Add driver for Maxim 77802 Power Management IC

[Javier Martinez Canillas]

Hello Mark,

On 06/16/2014 09:27 PM, Mark Brown wrote:
> On Mon, Jun 16, 2014 at 08:02:34PM +0200, Javier Martinez Canillas wrote:
> 
>> +- max77802,pmic-buck-dvs-gpios: The DVS GPIOs. We'll try to set these GPIOs
>> +  to match pmic-buck-default-dvs-idx at probe time if they are defined. If
>> +  some or all of these GPIOs are not defined it's assumed that the board has
>> +  any missing GPIOs hardwired to match pmic-buck-default-dvs-idx.
> 
> I can't tell from reading this what the property means exactly - I
> expect it is an array of the GPIOs in some order but that order isn't
> specified.
> 

Ok, I'll improve this property documentation.

>> +config MFD_MAX77802
>> +tristate "Maxim Integrated MAX77802 PMIC Support"
>> +depends on I2C=y
>> +select MFD_CORE
>> +select REGMAP_I2C
>> +select REGMAP_IRQ
>> +select IRQ_DOMAIN
>> +help
>> +  Say yes here to support for Maxim Integrated MAX77802.
>> +  This is a Power Management IC with RTC on chip.
>> +  This driver provides common support for accessing the device;
>> +  additional drivers must be enabled in order to use the functionality
>> +  of the device.
>> +
> 
> It is a bit unorthodox to put the build infrastructure in the same patch
> as the DT binding...
> 

I thought it was the opposite. That a DT binding document has to be added along
with the first user of the binding but I'll separate the DT doc in another patch
then if that is the right thing to do.

Thanks a lot for all your suggestions, I'll wait a little to see if there is
more feedback and repost a v3 addressing all the issues you pointed out.

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 v2 07/10] regulator: Add driver for Maxim 77802 PMIC regulators

[Javier Martinez Canillas]

Hello Mark,

Thanks a lot for your feedback.

On 06/16/2014 09:25 PM, Mark Brown wrote:
> On Mon, Jun 16, 2014 at 08:02:35PM +0200, Javier Martinez Canillas wrote:
> 
>> --- a/drivers/mfd/max77802.c
>> +++ b/drivers/mfd/max77802.c
>> @@ -37,6 +37,7 @@
>>  #include 
>>  
>>  static const struct mfd_cell max77802_devs[] = {
>> +{ .name = "max77802-pmic", },
>>  };
>>  
>>  static bool max77802_pmic_is_accessible_reg(struct device *dev,
> 
> Please don't do things like this, it makes it harder to apply your
> series.  Just register all the devices in the MFD when you add the MFD
> driver.
> 

Ok, I'll do that. After all mfd core just omits the devices that don't match.

>> +default:
>> +pr_warn("%s: regulator_suspend_mode : 0x%x not supported\n",
>> +rdev->desc->name, mode);
>> +return -EINVAL;
> 
> dev_warn().
> 

Ok.

>> +static void max77802_copy_reg(struct device *dev, struct regmap *regmap,
>> +  int from_reg, int to_reg)
>> +{
>> +int val;
>> +int ret;
>> +
>> +if (from_reg == to_reg)
>> +return;
>> +
>> +ret = regmap_read(regmap, from_reg, &val);
>> +if (!ret)
>> +ret = regmap_write(regmap, to_reg, val);
>> +
>> +if (ret)
>> +dev_warn(dev, "Copy err %d => %d (%d)\n",
>> + from_reg, to_reg, ret);
>> +}
> 
> Again, this looks like it should be generic.
> 

Yes, I missed this from your previous feedback, sorry about that.

I'll add a regmap_copy_reg() function to drivers/base/regmap/regmap.c instead.

>> +static int max77802_pmic_probe(struct platform_device *pdev)
>> +{
> 
>> +dev_dbg(&pdev->dev, "%s\n", __func__);
> 
> This isn't adding anything, just remove it - the core already logs
> probes if you want.
> 

Ok.

>> +config.dev = &pdev->dev;
> 
> Are you sure this shouldn't be the MFD?
> 

I just looked at regulator_register() and saw that it does rdev->dev.parent =
dev, so yes this has to be the MFD.

>> +for (i = 0; i < MAX77802_MAX_REGULATORS; i++) {
>> +struct regulator_dev *rdev;
>> +int id = pdata->regulators[i].id;
>> +
>> +config.init_data = pdata->regulators[i].initdata;
>> +config.of_node = pdata->regulators[i].of_node;
>> +
>> +max77802->opmode[id] = MAX77802_OPMODE_NORMAL;
> 
> Why isn't this being read from the hardware, this may lead to a
> configuration change the first time we pay attention?
> 

The original Chrome OS driver [0] had a "regulator-op-mode" property similar to
"op_mode" in Documentation/devicetree/bindings/regulator/s5m8767-regulator.txt
to specify the operating mode using DT.

But I removed that since I didn't want to have a specific property for what
appears to be a generic need. I wanted to re-post something along the lines of
what was discussed in [1] and add operating mode support to the generic
regulator code.

So, for now I thought it made sense to set the operating mode to normal on
probe() but I'll change it to read from the hardware if that is better.

I guess I should check in the datasheet if a sane default operating mode for
LDOs is expected when the chip is reseted or if this is left undefined and also
if the bootloader already set this.

Best regards,
Javier

[0]:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/Documentation/devicetree/bindings/mfd/max77xxx.txt
[1]: https://patchwork.kernel.org/patch/1855331/
--
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] devicetree: Add generic IOMMU device tree bindings

[Will Deacon]

On Tue, Jun 17, 2014 at 11:26:48AM +0100, Varun Sethi wrote:
> > The way we generally thought it would work was something like
> > this:
> >-u-boot/bootloader makes any static streamID allocation if needed,
> > sets a default streamID  (e.g. 0x0) in device and expresses
> > that in the device tree
> >-device tree would express relationship between devices
> > (including bus controllers) and the SMMU through mmu-masters
> > property
> >-u-boot would express the range of unused (or used) streamIDs via a
> > new
> > device tree property so the kernel SMMU driver knows what streamIDs
> > are
> > free
> >-in the SMMU driver a different vendor specific 'add_device' callback
> > could be used to handle our special cases where we need to set/change
> > the stream ID for devices added to a domain
> 
> Another possibility, could be to program the stream Id in the device
> registers (reference for the stream ID register can be obtained from the
> device tree) during device attach. This could be relevant in case of VFIO,
> when we are assigning multiple devices to a single VM. All the devices can
> share the same stream ID.

I think for simple masters (i.e. those that have all their StreamIDs under
control of one driver), then setting something during attach (or add?)
based on the DT could work pretty well. The other case is when we have
masters behind a bridge (such as a PCI RC). In this case, it might actually
be better to ask the bridge for the IDs and let it sort out the allocation
itself. That would also move the RequesterID -> StreamID mapping out of
the SMMU code.

What do you think?

Will
--
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] devicetree: Add generic IOMMU device tree bindings

[Varun Sethi]

> -Original Message-
> From: Yoder Stuart-B08248
> Sent: Tuesday, June 17, 2014 12:24 AM
> To: Will Deacon
> Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland;
> devicet...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Pawel
> Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren; linux-
> ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring; Kumar
> Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin; linux-arm-
> ker...@lists.infradead.org
> Subject: RE: [PATCH v2] devicetree: Add generic IOMMU device tree
> bindings
> 
> 
> 
> > -Original Message-
> > From: Will Deacon [mailto:will.dea...@arm.com]
> > Sent: Monday, June 16, 2014 12:04 PM
> > To: Yoder Stuart-B08248
> > Cc: Sethi Varun-B16395; Thierry Reding; Mark Rutland;
> > devicet...@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Pawel
> > Moll; Arnd Bergmann; Ian Campbell; Grant Grundler; Stephen Warren;
> > linux- ker...@vger.kernel.org; Marc Zyngier; Linux IOMMU; Rob Herring;
> > Kumar Gala; linux-te...@vger.kernel.org; Cho KyongHo; Dave P Martin;
> > linux-arm- ker...@lists.infradead.org
> > Subject: Re: [PATCH v2] devicetree: Add generic IOMMU device tree
> > bindings
> >
> > Hi Stuart,
> >
> > On Mon, Jun 16, 2014 at 05:56:32PM +0100, Stuart Yoder wrote:
> > > > Do you have use-cases where you really need to change these
> > > > mappings dynamically?
> > >
> > > Yes.  In the case of a PCI bus-- you may not know in advance how many
> > > PCI devices there are until you probe the bus.   We have another FSL
> > > proprietary bus we call the "fsl-mc" bus that is similar.
> >
> > For that case, though, you could still describe an algorithmic
> > transformation from RequesterID to StreamID which corresponds to a
> > fixed mapping.
> >
> > > Another thing to consider-- starting with SMMUv2, as you know, there
> > > is a new distributed architecture with multiple TBUs and a
> > > centralized TCU that walks the SMMU page tables.  So instead of
> > > sprinkling multiple SMMUs all over an SoC you now have the option a
> > > 1 central TCU and
> > sprinkling
> > > multiple TBUs around.   However, this means that the stream ID
> > namespace
> > > is now global and can be pretty limited.  In the SMMU implementation
> > > we have there are only 64 stream ID total for our Soc.  But we have
> > > many
> > more
> > > masters than that.
> > >
> > > So we look at stream IDs as really corresponding to an 'isolation
> > context'
> > > and not to a bus master.  An isolation context is the domain you are
> > > trying to isolate with the SMMU.  Devices that all belong to the
> > > same 'isolation context' can share the same stream ID, since they
> > > share the same domain and page tables.
> >
> > Ok, this is more compelling.
> >
> > > So, perhaps by default some/most SMMU masters may have a default
> > > stream
> > ID
> > > of 0x0 that is used by the host...and that could be represented
> > > statically in the device tree.
> > >
> > > But, we absolutely will need to dynamically set new stream IDs into
> > > masters when a new IOMMU 'domain' is created and devices
> > > are added to it.   All the devices in a domain will share
> > > the same stream ID.
> > >
> > > So whatever we do, let's please have an architecture flexible enough
> > > to allow for this.
> >
> > What is the software interface to the logic that assigns the StreamIDs?
> > Is
> > it part of the SMMU, or a separate device (or set of devices)?
> 
> For us at the hardware level there are a few different ways that the
> streamIDs can be set.  It is not part of the SMMU.  In the cases where
> there is simply
> 1 device to 1 streamID (e.g. USB controller) there is an SoC register
> where
> you just program the stream ID.   In the case of PCI, our PCI controller
> has a RequesterID-to-streamID table that you set via some PCI controller
> registers.
> 
> The way we generally thought it would work was something like
> this:
>-u-boot/bootloader makes any static streamID allocation if needed,
> sets a default streamID  (e.g. 0x0) in device and expresses
> that in the device tree
>-device tree would express relationship between devices
> (including bus controllers) and the SMMU through mmu-masters
> property
>-u-boot would express the range of unused (or used) streamIDs via a
> new
> device tree property so the kernel SMMU driver knows what streamIDs
> are
> free
>-in the SMMU driver a different vendor specific 'add_device' callback
> could be used to handle our special cases where we need to set/change
> the stream ID for devices added to a domain

Another possibility, could be to program the stream Id in the device registers 
(reference for the stream ID register can be obtained from the device tree) 
during device attach. This could be relevant in case of VFIO, when we are 
assigning multiple devices to a single VM. All the devices can share the same 
stream ID.

-Varun
--
To unsubscribe from this list:

Re: Boot warnings on exynos5420 based boards

[Srivatsa S. Bhat]

On 06/17/2014 03:03 PM, Sachin Kamat wrote:

>> Below is an updated patch, please let me know how it goes. (You'll have to
>> revert c47a9d7cca first, and then 56e692182, before trying this patch).
> 
> I am unable to apply your below patch on top of the above 2 reverts.
> Applying: CPU hotplug, smp: Execute any pending IPI callbacks before CPU 
> offline
> fatal: corrupt patch at line 106
> Patch failed at 0001 CPU hotplug, smp: Execute any pending IPI
> callbacks before CPU offline
> 
> Even with 'patch' I get the below failures:
> patching file kernel/smp.c
> Hunk #2 FAILED at 53.
> Hunk #3 FAILED at 179.
> 2 out of 3 hunks FAILED -- saving rejects to file kernel/smp.c.rej
> 

Hmm, weird. My mailer must have screwed it up.

Let's try again:

[In case this also doesn't work for you, please use this git tree in which
 I have reverted the 2 old commits and added this updated patch.

 git://github.com/srivatsabhat/linux.git ipi-offline-fix-v3
]


From: Srivatsa S. Bhat 

[PATCH] CPU hotplug, smp: Execute any pending IPI callbacks before CPU offline

There is a race between the CPU offline code (within stop-machine) and the
smp-call-function code, which can lead to getting IPIs on the outgoing CPU,
*after* it has gone offline.

Specifically, this can happen when using smp_call_function_single_async() to
send the IPI, since this API allows sending asynchronous IPIs from IRQ
disabled contexts. The exact race condition is described below.

During CPU offline, in stop-machine, we don't enforce any rule in the
_DISABLE_IRQ stage, regarding the order in which the outgoing CPU and the
other CPUs disable their local interrupts. Due to this, we can encounter a
situation in which an IPI is sent by one of the other CPUs to the outgoing CPU
(while it is *still* online), but the outgoing CPU ends up noticing it only
*after* it has gone offline.

  CPU 1 CPU 2
  (Online CPU)   (CPU going offline)

   Enter _PREPARE stage  Enter _PREPARE stage

 Enter _DISABLE_IRQ stage

   =
   Got a device interrupt, and | Didn't notice the IPI
   the interrupt handler sent an   | since interrupts were
   IPI to CPU 2 using  | disabled on this CPU.
   smp_call_function_single_async()|
   =

   Enter _DISABLE_IRQ stage

   Enter _RUN stage  Enter _RUN stage

  =
   Busy loop with interrupts  |  Invoke take_cpu_down()
   disabled.  |  and take CPU 2 offline
  =

   Enter _EXIT stage Enter _EXIT stage

   Re-enable interrupts  Re-enable interrupts

 The pending IPI is noted
 immediately, but alas,
 the CPU is offline at
 this point.

This of course, makes the smp-call-function IPI handler code running on CPU 2
unhappy and it complains about "receiving an IPI on an offline CPU".
One real example of the scenario on CPU 1 is the block layer's
complete-request call-path:
__blk_complete_request() [interrupt-handler]
raise_blk_irq()
smp_call_function_single_async()


However, if we look closely, the block layer does check that the target CPU is
online before firing the IPI. So in this case, it is actually the unfortunate
ordering/timing of events in the stop-machine phase that leads to receiving
IPIs after the target CPU has gone offline.

In reality, getting a late IPI on an offline CPU is not too bad by itself
(this can happen even due to hardware latencies in IPI send-receive). It is
a bug only if the target CPU really went offline without executing all the
callbacks queued on its list. (Note that a CPU is free to execute its pending
smp-call-function callbacks in a batch, without waiting for the corresponding
IPIs to arrive for each one of those callbacks).

So, fixing this issue can be broken up into two parts:
1. Ensure that a CPU goes offline only after executing all the callbacks
   queued on it.
2. Modify the warning condition in the smp-call-function IPI handler code
   such that it warns only if an offline CPU got an IPI *and* that CPU had
   gone offline with callbacks still pending in its queue.

Achieving part 1 is straight-forward - just flush (execute) all the queued
callbacks on the outgoing CPU in the CPU_DYING stage[1], including those
callbacks for which the sourc

Re: Boot warnings on exynos5420 based boards

[Sachin Kamat]

Hi Srivatsa,

Thanks for your prompt reply.

On Tue, Jun 17, 2014 at 2:48 PM, Srivatsa S. Bhat
 wrote:
> Hi Sachin,
>
> On 06/17/2014 01:39 PM, Sachin Kamat wrote:
>> Hi,
>>
>> I observe the below warnings while trying to boot Exynos5420 based boards
>> since yesterday's linux-next (next-20140616) using multi_v7_defconfig. Looks
>
> I guess you meant next-20140617.

I meant I started observing this warning next-20140616 onwards
(next-20140617 as well).

>
>> like it is triggered by the commit 56e6921829 ("CPU hotplug, smp:
>> flush any pending IPI callbacks before CPU offline"). Any ideas?
>>
>>
>> *
>> [0.046521] Exynos MCPM support installed
>> [0.048939] CPU1: Booted secondary processor
>> [0.065005] CPU1: update cpu_capacity 1535
>> [0.065011] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
>> [0.065660] CPU2: Booted secondary processor
>> [0.085005] CPU2: update cpu_capacity 1535
>> [0.085012] CPU2: thread -1, cpu 2, socket 0, mpidr 8002
>> [0.085662] CPU3: Booted secondary processor
>> [0.105005] CPU3: update cpu_capacity 1535
>> [0.105011] CPU3: thread -1, cpu 3, socket 0, mpidr 8003
>> [1.105031] CPU4: failed to come online
>> [1.105081] [ cut here ]
>> [1.105104] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
>> flush_smp_call_function_queue+0xc0/0x178()
>> [1.105112] Modules linked in:
>> [1.105129] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 3.15.0-next-20140616-2-g38f9385a061b #2035
>> [1.105157] [] (unwind_backtrace) from []
>> (show_stack+0x10/0x14)
>> [1.105179] [] (show_stack) from []
>> (dump_stack+0x8c/0x9c)
>> [1.105198] [] (dump_stack) from []
>> (warn_slowpath_common+0x70/0x8c)
>> [1.105216] [] (warn_slowpath_common) from []
>> (warn_slowpath_null+0x1c/0x24)
>> [1.105235] [] (warn_slowpath_null) from []
>> (flush_smp_call_function_queue+0xc0/0x178)
>> [1.105253] [] (flush_smp_call_function_queue) from
>> [] (hotplug_cfd+0x98/0xd8)
>> [1.105269] [] (hotplug_cfd) from []
>> (notifier_call_chain+0x44/0x84)
>> [1.105285] [] (notifier_call_chain) from []
>> (_cpu_up+0x120/0x170)
>> [1.105302] [] (_cpu_up) from [] (cpu_up+0x70/0x94)
>> [1.105319] [] (cpu_up) from [] (smp_init+0xac/0xb0)
>> [1.105337] [] (smp_init) from []
>> (kernel_init_freeable+0x90/0x1dc)
>> [1.105353] [] (kernel_init_freeable) from []
>> (kernel_init+0xc/0xe8)
>> [1.105368] [] (kernel_init) from []
>> (ret_from_fork+0x14/0x3c)
>> [1.105389] ---[ end trace bc66942e4ab63168 ]---
>
> Argh! I had put the switch-case handling for CPU_DYING at the 'wrong' place,
> since I hadn't noticed that CPU_UP_CANCELED silently falls-through to 
> CPU_DEAD.
> This is what happens when people don't explicitly write "fall-through" in the
> comments in a switch-case statement :-(
>
> Below is an updated patch, please let me know how it goes. (You'll have to
> revert c47a9d7cca first, and then 56e692182, before trying this patch).

I am unable to apply your below patch on top of the above 2 reverts.
Applying: CPU hotplug, smp: Execute any pending IPI callbacks before CPU offline
fatal: corrupt patch at line 106
Patch failed at 0001 CPU hotplug, smp: Execute any pending IPI
callbacks before CPU offline

Even with 'patch' I get the below failures:
patching file kernel/smp.c
Hunk #2 FAILED at 53.
Hunk #3 FAILED at 179.
2 out of 3 hunks FAILED -- saving rejects to file kernel/smp.c.rej

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

[PATCH 2/4] ARM: dts: exynos4412-odroidx: enable common hardware blocks

[Marek Szyprowski]

This patch adds support for common hardware modules available on all
Exynos4412-based Odroid boards, which already have complete support in
mainline kernel. This includes secure firmware calls, watchdog, g2d and
fimc (mem2mem) multimedia accelerators.

Signed-off-by: Marek Szyprowski 
---
 arch/arm/boot/dts/exynos4412-odroidx.dts | 35 
 1 file changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts 
b/arch/arm/boot/dts/exynos4412-odroidx.dts
index 31db28a4bb33..fda9ac23dd55 100644
--- a/arch/arm/boot/dts/exynos4412-odroidx.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidx.dts
@@ -22,6 +22,11 @@
reg = <0x4000 0x4000>;
};
 
+   firmware@0204F000 {
+   compatible = "samsung,secure-firmware";
+   reg = <0x0204F000 0x1000>;
+   };
+
leds {
compatible = "gpio-leds";
led1 {
@@ -68,10 +73,40 @@
regulator-boot-on;
};
 
+   watchdog@1006 {
+   status = "okay";
+   };
+
rtc@1007 {
status = "okay";
};
 
+   g2d@1080 {
+   status = "okay";
+   };
+
+   camera {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <>;
+
+   fimc_0: fimc@1180 {
+   status = "okay";
+   };
+
+   fimc_1: fimc@1181 {
+   status = "okay";
+   };
+
+   fimc_2: fimc@1182 {
+   status = "okay";
+   };
+
+   fimc_3: fimc@1183 {
+   status = "okay";
+   };
+   };
+
sdhci@1253 {
bus-width = <4>;
pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
-- 
1.9.2

--
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 4/4] ARM: dts: refactor Odroid DTS file and add support for Odroid X2 and U2/U3

[Marek Szyprowski]

This patch moves some parts of exynos4412-odroidx.dts to common
exynos4412-odroid-common.dtsi file and adds support for Odroid X2 and
U2/U3 boards. X2 is same as X, but it has faster SoC module (1.7GHz
instead of 1.4GHz), while U2/U3 differs from X2 by different way of
routing signals to host USB hub. It also lacks some hw modules not yet
supported by those dts files (i.e. LCD & touch panel).

Signed-off-by: Marek Szyprowski 
---
 arch/arm/boot/dts/Makefile  |   2 +
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 319 +++
 arch/arm/boot/dts/exynos4412-odroidu3.dts   |  49 
 arch/arm/boot/dts/exynos4412-odroidx.dts| 329 +---
 arch/arm/boot/dts/exynos4412-odroidx2.dts   |  23 ++
 5 files changed, 400 insertions(+), 322 deletions(-)
 create mode 100644 arch/arm/boot/dts/exynos4412-odroid-common.dtsi
 create mode 100644 arch/arm/boot/dts/exynos4412-odroidu3.dts
 create mode 100644 arch/arm/boot/dts/exynos4412-odroidx2.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 5986ff63b901..28b354936685 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -66,7 +66,9 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
exynos4210-smdkv310.dtb \
exynos4210-trats.dtb \
exynos4210-universal_c210.dtb \
+   exynos4412-odroidu3.dtb \
exynos4412-odroidx.dtb \
+   exynos4412-odroidx2.dtb \
exynos4412-origen.dtb \
exynos4412-smdk4412.dtb \
exynos4412-tiny4412.dtb \
diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi 
b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
new file mode 100644
index ..f793f3b8f0b9
--- /dev/null
+++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi
@@ -0,0 +1,319 @@
+/*
+ * Common definition for Hardkernel's Exynos4412 based ODROID-X/X2/U2/U3 boards
+ * device tree source
+ *
+ * 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 "exynos4412.dtsi"
+
+/ {
+   firmware@0204F000 {
+   compatible = "samsung,secure-firmware";
+   reg = <0x0204F000 0x1000>;
+   };
+
+   mmc@1255 {
+   pinctrl-0 = <&sd4_clk &sd4_cmd &sd4_bus4 &sd4_bus8>;
+   pinctrl-names = "default";
+   vmmc-supply = <&ldo20_reg &buck8_reg>;
+   status = "okay";
+
+   num-slots = <1>;
+   supports-highspeed;
+   broken-cd;
+   card-detect-delay = <200>;
+   samsung,dw-mshc-ciu-div = <3>;
+   samsung,dw-mshc-sdr-timing = <2 3>;
+   samsung,dw-mshc-ddr-timing = <1 2>;
+
+   slot@0 {
+   reg = <0>;
+   bus-width = <8>;
+   };
+   };
+
+   watchdog@1006 {
+   status = "okay";
+   };
+
+   rtc@1007 {
+   status = "okay";
+   };
+
+   g2d@1080 {
+   status = "okay";
+   };
+
+   camera {
+   status = "okay";
+   pinctrl-names = "default";
+   pinctrl-0 = <>;
+
+   fimc_0: fimc@1180 {
+   status = "okay";
+   };
+
+   fimc_1: fimc@1181 {
+   status = "okay";
+   };
+
+   fimc_2: fimc@1182 {
+   status = "okay";
+   };
+
+   fimc_3: fimc@1183 {
+   status = "okay";
+   };
+   };
+
+   sdhci@1253 {
+   bus-width = <4>;
+   pinctrl-0 = <&sd2_clk &sd2_cmd &sd2_cd &sd2_bus4>;
+   pinctrl-names = "default";
+   vmmc-supply = <&ldo4_reg &ldo21_reg>;
+   status = "okay";
+   };
+
+   serial@1380 {
+   status = "okay";
+   };
+
+   serial@1381 {
+   status = "okay";
+   };
+
+   fixed-rate-clocks {
+   xxti {
+   compatible = "samsung,clock-xxti";
+   clock-frequency = <0>;
+   };
+
+   xusbxti {
+   compatible = "samsung,clock-xusbxti";
+   clock-frequency = <2400>;
+   };
+   };
+
+   i2c@1386 {
+   pinctrl-0 = <&i2c0_bus>;
+   pinctrl-names = "default";
+   status = "okay";
+
+   usb3503: usb3503@08 {
+   compatible = "smsc,usb3503";
+   reg = <0x08>;
+
+   intn-gpios = <&gpx3 0 0>;
+   connect-gpios = <&gpx3 4 0>;
+   reset-gpios = <&gpx3 5 0>;
+   initial-mode = <1>;
+   };
+
+   max77686: pmic@09

[PATCH 0/4] Add Exynos4412 based Odroid X2 and U2/U3/U3+ support

[Marek Szyprowski]

Hello,

This is the initial patch series adding support for Exynos 4412 based
Odroid X2 and U2/U3/U3+ boards and improving support for Odroid X.

Complete USB support for Odroid U2/U3/U3+ still requires some fixes in
Exynos4 USB2 Phy driver and clock driver for CLKOUT:
http://www.spinics.net/lists/linux-usb/msg108028.html
http://www.spinics.net/lists/kernel/msg1743815.html 
The above changes however don't affect Odroid DTS files, but without
them, usb3503 hub is not yet functional.

Support for audio codec will be posted separately by Sylwester Nawrocki
soon. Support for HDMI video output will be also posted separately
together with the required ExynosDRM-HDMI fixes.

If you are interested in more complete and open-source Odroid board
support, please also check the latest patches for u-boot project:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/188295/focus=188610

Best regards
Marek Szyprowski
Samsung R&D Institute Poland

Marek Szyprowski (4):
  ARM: dts: exynos4: add port sub-nodes to exynos usb host modules
  ARM: dts: exynos4412-odroidx: enable common hardware blocks
  ARM: dts: exynos4412-odroidx: add support for USB (phy, host, device)
  ARM: dts: refactor Odroid DTS file and add support for Odroid X2 and
U2/U3

 arch/arm/boot/dts/Makefile  |   2 +
 arch/arm/boot/dts/exynos4.dtsi  |  24 ++
 arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 319 
 arch/arm/boot/dts/exynos4412-odroidu3.dts   |  49 
 arch/arm/boot/dts/exynos4412-odroidx.dts| 264 +---
 arch/arm/boot/dts/exynos4412-odroidx2.dts   |  23 ++
 6 files changed, 424 insertions(+), 257 deletions(-)
 create mode 100644 arch/arm/boot/dts/exynos4412-odroid-common.dtsi
 create mode 100644 arch/arm/boot/dts/exynos4412-odroidu3.dts
 create mode 100644 arch/arm/boot/dts/exynos4412-odroidx2.dts

-- 
1.9.2
--
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/4] ARM: dts: exynos4412-odroidx: add support for USB (phy, host, device)

[Marek Szyprowski]

From: Kamil Debski 

This patch adds basic support for USB modules (host and device) on
OdroidX board.

Signed-off-by: Kamil Debski 
Signed-off-by: Marek Szyprowski 
---
 arch/arm/boot/dts/exynos4412-odroidx.dts | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts 
b/arch/arm/boot/dts/exynos4412-odroidx.dts
index fda9ac23dd55..1a067cc00072 100644
--- a/arch/arm/boot/dts/exynos4412-odroidx.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidx.dts
@@ -148,6 +148,16 @@
pinctrl-names = "default";
status = "okay";
 
+   usb3503@08 {
+   compatible = "smsc,usb3503";
+   reg = <0x08>;
+
+   intn-gpios = <&gpx3 0 0>;
+   connect-gpios = <&gpx3 4 0>;
+   reset-gpios = <&gpx3 5 0>;
+   initial-mode = <1>;
+   };
+
max77686: pmic@09 {
compatible = "maxim,max77686";
reg = <0x09>;
@@ -338,4 +348,24 @@
};
};
};
+
+   exynos-usbphy@125B {
+   status = "okay";
+   };
+
+   hsotg@1248 {
+   status = "okay";
+   vusb_d-supply = <&ldo15_reg>;
+   vusb_a-supply = <&ldo12_reg>;
+   };
+
+   ehci@1258 {
+   status = "okay";
+   port@1 {
+   status = "okay";
+   };
+   port@2 {
+   status = "okay";
+   };
+   };
 };
-- 
1.9.2

--
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/4] ARM: dts: exynos4: add port sub-nodes to exynos usb host modules

[Marek Szyprowski]

This patch adds port sub-nodes to exynos4 ehci and ohci modules, which
are required by recently merged new exynos4 usb2 phy support.

Signed-off-by: Marek Szyprowski 
---
 arch/arm/boot/dts/exynos4.dtsi | 24 
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index b8ece4be41ca..c91284441694 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -322,6 +322,23 @@
clocks = <&clock CLK_USB_HOST>;
clock-names = "usbhost";
status = "disabled";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   port@0 {
+   reg = <0>;
+   phys = <&exynos_usbphy 1>;
+   status = "disabled";
+   };
+   port@1 {
+   reg = <1>;
+   phys = <&exynos_usbphy 2>;
+   status = "disabled";
+   };
+   port@2 {
+   reg = <2>;
+   phys = <&exynos_usbphy 3>;
+   status = "disabled";
+   };
};
 
ohci@1259 {
@@ -331,6 +348,13 @@
clocks = <&clock CLK_USB_HOST>;
clock-names = "usbhost";
status = "disabled";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   port@0 {
+   reg = <0>;
+   phys = <&exynos_usbphy 1>;
+   status = "disabled";
+   };
};
 
i2s1: i2s@1396 {
-- 
1.9.2

--
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: Boot warnings on exynos5420 based boards

[Srivatsa S. Bhat]

Hi Sachin,

On 06/17/2014 01:39 PM, Sachin Kamat wrote:
> Hi,
> 
> I observe the below warnings while trying to boot Exynos5420 based boards
> since yesterday's linux-next (next-20140616) using multi_v7_defconfig. Looks

I guess you meant next-20140617.

> like it is triggered by the commit 56e6921829 ("CPU hotplug, smp:
> flush any pending IPI callbacks before CPU offline"). Any ideas?
> 
> 
> *
> [0.046521] Exynos MCPM support installed
> [0.048939] CPU1: Booted secondary processor
> [0.065005] CPU1: update cpu_capacity 1535
> [0.065011] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
> [0.065660] CPU2: Booted secondary processor
> [0.085005] CPU2: update cpu_capacity 1535
> [0.085012] CPU2: thread -1, cpu 2, socket 0, mpidr 8002
> [0.085662] CPU3: Booted secondary processor
> [0.105005] CPU3: update cpu_capacity 1535
> [0.105011] CPU3: thread -1, cpu 3, socket 0, mpidr 8003
> [1.105031] CPU4: failed to come online
> [1.105081] [ cut here ]
> [1.105104] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
> flush_smp_call_function_queue+0xc0/0x178()
> [1.105112] Modules linked in:
> [1.105129] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 3.15.0-next-20140616-2-g38f9385a061b #2035
> [1.105157] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [1.105179] [] (show_stack) from []
> (dump_stack+0x8c/0x9c)
> [1.105198] [] (dump_stack) from []
> (warn_slowpath_common+0x70/0x8c)
> [1.105216] [] (warn_slowpath_common) from []
> (warn_slowpath_null+0x1c/0x24)
> [1.105235] [] (warn_slowpath_null) from []
> (flush_smp_call_function_queue+0xc0/0x178)
> [1.105253] [] (flush_smp_call_function_queue) from
> [] (hotplug_cfd+0x98/0xd8)
> [1.105269] [] (hotplug_cfd) from []
> (notifier_call_chain+0x44/0x84)
> [1.105285] [] (notifier_call_chain) from []
> (_cpu_up+0x120/0x170)
> [1.105302] [] (_cpu_up) from [] (cpu_up+0x70/0x94)
> [1.105319] [] (cpu_up) from [] (smp_init+0xac/0xb0)
> [1.105337] [] (smp_init) from []
> (kernel_init_freeable+0x90/0x1dc)
> [1.105353] [] (kernel_init_freeable) from []
> (kernel_init+0xc/0xe8)
> [1.105368] [] (kernel_init) from []
> (ret_from_fork+0x14/0x3c)
> [1.105389] ---[ end trace bc66942e4ab63168 ]---

Argh! I had put the switch-case handling for CPU_DYING at the 'wrong' place,
since I hadn't noticed that CPU_UP_CANCELED silently falls-through to CPU_DEAD.
This is what happens when people don't explicitly write "fall-through" in the
comments in a switch-case statement :-(

Below is an updated patch, please let me know how it goes. (You'll have to
revert c47a9d7cca first, and then 56e692182, before trying this patch).

[c47a9d7cca - CPU hotplug, smp: Execute any pending IPI callbacks before CPU
  offline]
[56e692182  - CPU hotplug, smp: flush any pending IPI callbacks before CPU
  offline]

Andrew, can you please use this patch instead?

Thanks a lot!

--

From: Srivatsa S. Bhat 
[PATCH] CPU hotplug, smp: Execute any pending IPI callbacks before CPU offline

There is a race between the CPU offline code (within stop-machine) and the
smp-call-function code, which can lead to getting IPIs on the outgoing CPU,
*after* it has gone offline.

Specifically, this can happen when using smp_call_function_single_async() to
send the IPI, since this API allows sending asynchronous IPIs from IRQ
disabled contexts. The exact race condition is described below.

During CPU offline, in stop-machine, we don't enforce any rule in the
_DISABLE_IRQ stage, regarding the order in which the outgoing CPU and the
other CPUs disable their local interrupts. Due to this, we can encounter a
situation in which an IPI is sent by one of the other CPUs to the outgoing CPU
(while it is *still* online), but the outgoing CPU ends up noticing it only
*after* it has gone offline.

  CPU 1 CPU 2
  (Online CPU)   (CPU going offline)

   Enter _PREPARE stage  Enter _PREPARE stage

 Enter _DISABLE_IRQ stage

   =
   Got a device interrupt, and | Didn't notice the IPI
   the interrupt handler sent an   | since interrupts were
   IPI to CPU 2 using  | disabled on this CPU.
   smp_call_function_single_async()|
   =

   Enter _DISABLE_IRQ stage

   Enter _RUN stage  

Re: [PATCH v3 5/7] mfd: cros_ec: Sync to the latest cros_ec_commands.h from EC sources

[Paul Bolle]

Doug,

On Fri, 2014-06-13 at 08:22 -0700, Doug Anderson wrote:
> On Fri, Jun 13, 2014 at 1:08 AM, Paul Bolle  wrote:
> > On Wed, 2014-06-11 at 08:11 -0700, Doug Anderson wrote:
> >> This is a config option on the ChromeOS EC
> >> .  Doing a
> >> grep there:
> >>
> >> board/samus/board.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> driver/battery/samus.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> driver/battery/samus.c:#endif   /* CONFIG_CHARGER_PROFILE_OVERRIDE */
> >> include/config.h:#undef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> include/ec_commands.h:  /* Range for CONFIG_CHARGER_PROFILE_OVERRIDE 
> >> params */
> >> test/test_config.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
> >
> > I see. So this is not a Kconfig macro but a general macro with a CONFIG_
> > prefix. There are quite a bit of those in the tree already, but still,
> > would another prefix also do?
> 
> Given that it's an entirely separate project and this is a valid
> CONFIG option in that project, it seems a lot to ask them not to use
> the CONFIG_ prefix.  Also: the part you are objecting to is only a
> comment, right?

So all the hits you quoted above are actually from code that's never
going to be included in the kernel tree, right? If so, then yes, we're
only discussing a single comment.

> We could certainly add extra wording in the comment to make it obvious
> that this is a CONFIG option for the EC and not the kernel.  Would
> that be enough?  ...or are you trying to use some scripts to
> automatically process files to look for CONFIG options?

Yes, I'm using a script to check for Kconfig macros, among other things.
It doesn't care about comments (because every now and then mistakes are
made in comments too, and some of those can get surprisingly confusing).

Anyhow, the CONFIG_ prefix used in the kernel tree is quite generic, but
we're stuck with it. Would it be bothersome to drop it in that comment?
Mentioning a preprocessor macro from a separate project is a bit
confusing to begin with. How is one supposed to know that this is a
reference to something out of tree?

So, in summary, while we're apparently only discussing a single comment,
I would appreciate it if it could be reworded, preferably by dropping
that the CONFIG_ prefix. But other people might care very little, as
they don't share this particular pet peeve.

Thanks,


Paul Bolle

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

Boot warnings on exynos5420 based boards

[Sachin Kamat]

Hi,

I observe the below warnings while trying to boot Exynos5420 based boards
since yesterday's linux-next (next-20140616) using multi_v7_defconfig. Looks
like it is triggered by the commit 56e6921829 ("CPU hotplug, smp:
flush any pending IPI callbacks before CPU offline"). Any ideas?


*
[0.046521] Exynos MCPM support installed
[0.048939] CPU1: Booted secondary processor
[0.065005] CPU1: update cpu_capacity 1535
[0.065011] CPU1: thread -1, cpu 1, socket 0, mpidr 8001
[0.065660] CPU2: Booted secondary processor
[0.085005] CPU2: update cpu_capacity 1535
[0.085012] CPU2: thread -1, cpu 2, socket 0, mpidr 8002
[0.085662] CPU3: Booted secondary processor
[0.105005] CPU3: update cpu_capacity 1535
[0.105011] CPU3: thread -1, cpu 3, socket 0, mpidr 8003
[1.105031] CPU4: failed to come online
[1.105081] [ cut here ]
[1.105104] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
flush_smp_call_function_queue+0xc0/0x178()
[1.105112] Modules linked in:
[1.105129] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
3.15.0-next-20140616-2-g38f9385a061b #2035
[1.105157] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[1.105179] [] (show_stack) from []
(dump_stack+0x8c/0x9c)
[1.105198] [] (dump_stack) from []
(warn_slowpath_common+0x70/0x8c)
[1.105216] [] (warn_slowpath_common) from []
(warn_slowpath_null+0x1c/0x24)
[1.105235] [] (warn_slowpath_null) from []
(flush_smp_call_function_queue+0xc0/0x178)
[1.105253] [] (flush_smp_call_function_queue) from
[] (hotplug_cfd+0x98/0xd8)
[1.105269] [] (hotplug_cfd) from []
(notifier_call_chain+0x44/0x84)
[1.105285] [] (notifier_call_chain) from []
(_cpu_up+0x120/0x170)
[1.105302] [] (_cpu_up) from [] (cpu_up+0x70/0x94)
[1.105319] [] (cpu_up) from [] (smp_init+0xac/0xb0)
[1.105337] [] (smp_init) from []
(kernel_init_freeable+0x90/0x1dc)
[1.105353] [] (kernel_init_freeable) from []
(kernel_init+0xc/0xe8)
[1.105368] [] (kernel_init) from []
(ret_from_fork+0x14/0x3c)
[1.105389] ---[ end trace bc66942e4ab63168 ]---
[2.105047] CPU5: failed to come online
[2.105073] [ cut here ]
[2.105091] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
flush_smp_call_function_queue+0xc0/0x178()
[2.105099] Modules linked in:
[2.105114] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
3.15.0-next-20140616-2-g38f9385a061b #2035
[2.105135] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[2.105153] [] (show_stack) from []
(dump_stack+0x8c/0x9c)
[2.105170] [] (dump_stack) from []
(warn_slowpath_common+0x70/0x8c)
[2.105187] [] (warn_slowpath_common) from []
(warn_slowpath_null+0x1c/0x24)
[2.105205] [] (warn_slowpath_null) from []
(flush_smp_call_function_queue+0xc0/0x178)
[2.105222] [] (flush_smp_call_function_queue) from
[] (hotplug_cfd+0x98/0xd8)
[2.105237] [] (hotplug_cfd) from []
(notifier_call_chain+0x44/0x84)
[2.105252] [] (notifier_call_chain) from []
(_cpu_up+0x120/0x170)
[2.105268] [] (_cpu_up) from [] (cpu_up+0x70/0x94)
[2.105285] [] (cpu_up) from [] (smp_init+0xac/0xb0)
[2.105301] [] (smp_init) from []
(kernel_init_freeable+0x90/0x1dc)
[2.105316] [] (kernel_init_freeable) from []
(kernel_init+0xc/0xe8)
[2.105330] [] (kernel_init) from []
(ret_from_fork+0x14/0x3c)
[2.105339] ---[ end trace bc66942e4ab63169 ]---
[3.105064] CPU6: failed to come online
[3.105089] [ cut here ]
[3.105107] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
flush_smp_call_function_queue+0xc0/0x178()
[3.105115] Modules linked in:
[3.105131] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
3.15.0-next-20140616-2-g38f9385a061b #2035
[3.105150] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[3.105168] [] (show_stack) from []
(dump_stack+0x8c/0x9c)
[3.105185] [] (dump_stack) from []
(warn_slowpath_common+0x70/0x8c)
[3.105202] [] (warn_slowpath_common) from []
(warn_slowpath_null+0x1c/0x24)
[3.105220] [] (warn_slowpath_null) from []
(flush_smp_call_function_queue+0xc0/0x178)
[3.105237] [] (flush_smp_call_function_queue) from
[] (hotplug_cfd+0x98/0xd8)
[3.105252] [] (hotplug_cfd) from []
(notifier_call_chain+0x44/0x84)
[3.105267] [] (notifier_call_chain) from []
(_cpu_up+0x120/0x170)
[3.105283] [] (_cpu_up) from [] (cpu_up+0x70/0x94)
[3.105299] [] (cpu_up) from [] (smp_init+0xac/0xb0)
[3.105315] [] (smp_init) from []
(kernel_init_freeable+0x90/0x1dc)
[3.105330] [] (kernel_init_freeable) from []
(kernel_init+0xc/0xe8)
[3.105344] [] (kernel_init) from []
(ret_from_fork+0x14/0x3c)
[3.105353] ---[ end trace bc66942e4ab6316a ]---
[4.105081] CPU7: failed to come online
[4.105106] [ cut here ]
[4.105124] WARNING: CPU: 0 PID: 1 at kernel/smp.c:228
flush_smp_call_function_queue+0xc0/0x178()
[4.1051