Re: [PATCH V4 22/30] thermal: exynos: Add support for exynos5440 TMU sensor.

[jonghwa3 . lee]

On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:

> This patch modifies TMU controller to add changes needed to work with
> exynos5440 platform. This sensor registers 3 instance of the tmu controller
> with the thermal zone and hence reports 3 temperature output. This controller
> supports upto five trip points. For critical threshold the driver uses the
> core driver thermal framework for shutdown.
> 
> Acked-by: Kukjin Kim 
> Signed-off-by: Amit Daniel Kachhap 
> ---
>  .../devicetree/bindings/thermal/exynos-thermal.txt |   28 -
>  drivers/thermal/samsung/exynos_tmu.c   |   43 +--
>  drivers/thermal/samsung/exynos_tmu.h   |6 +++
>  drivers/thermal/samsung/exynos_tmu_data.h  |2 +
>  4 files changed, 72 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt 
> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> index 535fd0e..970eeba 100644
> --- a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt
> @@ -6,13 +6,16 @@
>  "samsung,exynos4412-tmu"
>  "samsung,exynos4210-tmu"
>  "samsung,exynos5250-tmu"
> +"samsung,exynos5440-tmu"
>  - interrupt-parent : The phandle for the interrupt controller
> -- reg : Address range of the thermal registers
> +- reg : Address range of the thermal registers. For exynos5440-tmu which has 
> 3
> + instances of TMU, 2 set of register has to supplied. First set belongs
> + to each instance of TMU and second set belongs to common TMU registers.
>  - interrupts : Should contain interrupt for thermal system
>  - clocks : The main clock for TMU device
>  - clock-names : Thermal system clock name
>  
> -Example:
> +Example 1):
>  
>   tmu@100C {
>   compatible = "samsung,exynos4412-tmu";
> @@ -23,3 +26,24 @@ Example:
>   clock-names = "tmu_apbif";
>   status = "disabled";
>   };
> +
> +Example 2):
> +
> + tmuctrl_0: tmuctrl@160118 {
> + compatible = "samsung,exynos5440-tmu";
> + reg = <0x160118 0x230>, <0x160368 0x10>;
> + interrupts = <0 58 0>;
> + clocks = <&clock 21>;
> + clock-names = "tmu_apbif";
> + };
> +
> +Note: For multi-instance tmu each instance should have an alias correctly
> +numbered in "aliases" node.
> +
> +Example:
> +
> +aliases {
> + tmuctrl0 = &tmuctrl_0;
> + tmuctrl1 = &tmuctrl_1;
> + tmuctrl2 = &tmuctrl_2;
> +};
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index 7f7b1cf..7ca9c4d 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -185,9 +185,11 @@ static int exynos_tmu_initialize(struct platform_device 
> *pdev)
>   reg->threshold_th0 + i * sizeof(reg->threshold_th0));
>  
>   writel(reg->inten_rise_mask, data->base + reg->tmu_intclear);
> - } else if (data->soc == SOC_ARCH_EXYNOS) {
> + } else if (data->soc == SOC_ARCH_EXYNOS ||
> + data->soc == SOC_ARCH_EXYNOS5440) {
>   /* Write temperature code for rising and falling threshold */
> - for (i = 0; i < trigger_levs; i++) {
> + for (i = 0;
> + i < trigger_levs && i < EXYNOS_MAX_TRIGGER_PER_REG; i++) {
>   threshold_code = temp_to_code(data,
>   pdata->trigger_levels[i]);
>   if (threshold_code < 0) {
> @@ -218,7 +220,30 @@ static int exynos_tmu_initialize(struct platform_device 
> *pdev)
>   writel((reg->inten_rise_mask << reg->inten_rise_shift) |
>   (reg->inten_fall_mask << reg->inten_fall_shift),
>   data->base + reg->tmu_intclear);
> +
> + /* if 5th threshold limit is also present, use TH2 register */
> + i = EXYNOS_MAX_TRIGGER_PER_REG;
> + if (pdata->trigger_levels[i]) {
> + threshold_code = temp_to_code(data,
> + pdata->trigger_levels[i]);
> + if (threshold_code < 0) {
> + ret = threshold_code;
> + goto out;
> + }
> + rising_threshold =
> + threshold_code << reg->threshold_th3_l0_shift;
> + writel(rising_threshold,
> + data->base + reg->threshold_th2);
> + if (pdata->trigger_type[i] == HW_TRIP) {
> + con = readl(data->base + reg->tmu_ctrl);
> + con |= (1 << reg->therm_trip_en_shift);
> + writel(con, data->base + reg->tmu_ctrl);
> + }
> + }
>

Re: [PATCH 6/6] pinctrl: exynos: Handle suspend/resume of GPIO EINT registers

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 1:34 PM, Tomasz Figa  wrote:
>> Slight nit to add this before the call to irq_domain_add_linear().
>> demv() will handle freeing your memory but nothing will handle undoing
>> the irq_domain_add_linear() if you return an error.
>
> I'm a bit sceptical when it is about error handling in such cases.
> Basically if interrupt initialization fails, something is seriously wrong,
> either with your kernel config or with some code.
>
> Since such case has been already unhandled in the driver (with nr_banks >
> 1 = always), so I didn't bother to add any undoing here.

Yeah, not all drivers handle it well.  I'm always surprised by the
number of drivers that seem have it right, though.  Certainly it seems
awfully unlikely that allocating a small number of bytes in a probe
function would fail.

...but changing the order doesn't hurt anything and would make it more
correct, even if it's not fully correct.


>> Optional: debug statements:
>>
>> pr_debug("%s: con %#010x => %#010x\n", bank->name,
>>   readl(regs + EXYNOS_GPIO_ECON_OFFSET + bank->eint_offset),
>>   save->eint_con);
>> pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name,
>>   readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset),
>>   save->eint_fltcon0);
>> pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name,
>>   readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset + 4),
>>   save->eint_fltcon1);
>
> OK. I wonder if this could be added in a separate patch or I should rather
> send v2 on Monday?

Definitely optional to add these, so up to you whether to spin the
patch, ignore my suggestion, or do a separate patch.  :)


Thanks for sending all of these up, by the way!  If things look good
next week I'll probably revert my local version of this (the V2 of my
original series) and pull in your series to keep us on the same page.
:)


-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: Pulls and drive strengths in the pinctrl world

[Tomasz Figa]

Hi Jean-Christophe,

On Friday 17 of May 2013 14:26:25 Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 18:22 Wed 15 May , Stephen Warren wrote:
> > On 05/15/2013 06:13 PM, Tomasz Figa wrote:
> > > On Wednesday 15 of May 2013 16:55:37 Doug Anderson wrote:
> > >> Tomasz / Linus,
> > >> 
> > >> On Wed, May 15, 2013 at 3:06 PM, Tomasz Figa
> > >> 
> > > 
> > > wrote:
> > >>> Yes. I don't like the current way too much either, duplication
> > >>> being
> > >>> one of the reasons.
> > >> 
> > >> Do you have any other ideas?  It sounds like Linus didn't like my
> > >> suggestion and makes some good points...
> > > 
> > > I don't have anything interesting at the moment. It's a bit late now
> > > here (2 AM), so I'm going to get some sleep first.
> > > 
> > > Also after reading Stephen's reply, I'm wondering if hogging
> > > wouldn't
> > > solve the problem indeed. (It might have to be fixed on
> > > pinctrl-samsung
> > > first, as last time I tried to use it, it caused some errors from
> > > pinctrl core, but haven't time to track them down, as it wasn't
> > > anything important at that time).
> > 
> > One issue I noticed with the DT fragments earlier in this thread. It
> > looks like hogs in the Samsung pinctrl bingings end up looking like:
> > 
> > pinctrl {
> > 
> > pina {
> > 
> > samsung,pins = ;
> > samsung,pin-function = <0xf>;
> > samsung,pin-pud = <0>;
> > ...
> 
> I have a huge issue here that we had on at91 too
> 
> we are going to have a huge numbet of node
> 
> and on at91 we handle the pin the same way as samsung
> and ST have also a similiar IP
> 
> so I'll prefer to reuse the AT91 DT bindings
> 
> as said by Linus I just push a cleanup of the magic by using Macro
> which make it really readable now
> 
> some extract of the sama5 pinctrl
> 
>   mmc0 {
>   pinctrl_mmc0_clk_cmd_dat0: mmc0_clk_cmd_dat0 {
>   atmel,pins =
>*/ AT91_PIOD 0 AT91_PERIPH_A AT91_PINCTRL_PULL_UP /* PD0 periph A
> MCI0_CDA with pullup */ AT91_PIOD 1 AT91_PERIPH_A
> AT91_PINCTRL_PULL_UP>;/* PD1 periph A MCI0_DA0 with pullup */ };
>   pinctrl_mmc0_dat1_3: mmc0_dat1_3 {
>   atmel,pins =
>MCI0_DA1 with pullup */ AT91_PIOD 3 AT91_PERIPH_A
> AT91_PINCTRL_PULL_UP  /* PD3 periph A MCI0_DA2 with pullup */ AT91_PIOD
> 4 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>;/* PD4 periph A MCI0_DA3 
with
> pullup */ };
>   pinctrl_mmc0_dat4_7: mmc0_dat4_7 {
>   atmel,pins =
>MCI0_DA4 with pullup, conflicts with TIOA0, PWMH2 */ AT91_PIOD 6
> AT91_PERIPH_A AT91_PINCTRL_PULL_UP/* PD6 periph A MCI0_DA5 with
> pullup, conflicts with TIOB0, PWML2 */ AT91_PIOD 7 AT91_PERIPH_A
> AT91_PINCTRL_PULL_UP  /* PD7 periph A MCI0_DA6 with pullup, conlicts
> with TCLK0, PWMH3 */ AT91_PIOD 8 AT91_PERIPH_A
> AT91_PINCTRL_PULL_UP>;/* PD8 periph A MCI0_DA7 with pullup, 
conflicts
> with PWML3 */ };
>   };
> 
> of sam9g45
> 
>   i2c_gpio2 {
>   pinctrl_i2c_gpio2: i2c_gpio2-0 {
>   atmel,pins =
>multidrive I2C2 data */ AT91_PIOB 5 AT91_PERIPH_GPIO
> AT91_PINCTRL_MULTI_DRIVE>;/* PB5 gpio multidrive I2C2 clock */ };
>   };
> 
> so we could share the c code too

I'd have a question with regard to AT91 bindings.

Using Samsung bindings we don't need to specify all configuration options 
for a pin, only those that are relevant for the platform. Do your bindings 
allow this?

Apparently AT91 has less configurable things and those available are 
usually always configured together so it's not a problem. But on our SoCs 
we have a bit more of them:
- function (input, output, special functions)
- pull-down/-up
- driver strength
- power down mode function (input, output low, output high, retention)
- power down mode pull-down/-up
- one could argue that default output value could be set this way as well, 
by adding samsung,pin-value property.

Best regards,
Tomasz

> Best Regards,
> J,
> 
> > };
> > pinp {
> > 
> > samsung,pins = ;
> > samsung,pin-function = <0xe>;
> > samsung,pin-pud = <1>;
> > ...
> > 
> > };
> > pinx {
> > 
> > samsung,pins = ;
> > samsung,pin-function = <0xd>;
> > samsung,pin-pud = <2>;
> > ...
> > 
> > };
> > 
> > pinctrl-names = "default";
> > pinctrl-0 = <&pina &pinp &pinx>;
> > 
> > };
> > 
> > That pinctrl-0 property could get rather large (hard to
> > write/maintain,
> > unwieldy) if it needs to set up lots of different configurations.
> > That's why I made the equivalent Tegra bindings be:
> > 
> > pinctrl {
> > 
> > pins_default {
> > 
> > pina {
> > 
> > samsung,pins = ;
> > samsung,pin-function = <0xf>;
> > samsung,pin-pud = <0>;
> > ...
>

Re: [PATCH 3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

[Tomasz Figa]

On Friday 17 of May 2013 13:56:39 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 1:23 PM, Tomasz Figa  
wrote:
> >> I'd rather see you modify patch set #2 to provide some function to
> >> retrieve just the eint mask and then use it here.  Your patch removes
> >> this test and doesn't replace it with anything.  If you moved this
> >> test to pinctrl directly you'd lose the test against intallow.
> > 
> > Well, looking from the perspective of status before my patch, it just
> > bypasses a test that is incorrect on DT-enabled platforms.
> 
> True.  What was there before was broken and this avoids the broken code.
> > I agree that this test is rather reasonable to have, but it would
> > require defining a new interface and moving all platforms to it,
> > which for now would be a bit of overkill.
> 
> It seems like you could use the same type of solution as patch set #2?
> 
> ...oh, but that's only for exynos!  I guess we would need something
> similar for other platforms.  ...and until we do those other platforms
> (including S3C64xx, I think) are still using the old
> s3c_irqwake_eintmask, right?

Correct.

Also as an extra side note, intallow is used here as a mask of valid 
eintmask bits on given platform. On Exynos SoCs there are 32 EINTs, so 
this extra mask is not needed.

> ...so overall your patch series only fully fixes exynos, though it
> does make other platforms less broken which is a plus.  :)
> 
> > IMHO a separate series that sanitizes the whole PM handling in plat-
> > samsung, including a rework of this check to make it cover all
> > platforms in a generic and multiplatform-friendly way. What do you
> > think?
> Sure, I think that would be OK.
> 
> >> Ah, the important part here is the "saved", not the "save"!  The
> >> "save" should get a NULL chip for all GPIOs and effectively be a
> >> no-op.
> >> 
> >> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
> >> "saved" seems to transition things over to powerdown mode.  Hopefully
> >> a future patch of yours adds that back for those platforms in the
> >> pinmux world.  ...same for restore.
> > 
> > S3C64xx can be switched to power down pin configuration manually, but
> > if you don't do it, it will activate it automatically after entering
> > sleep mode.
> 
> How would restore work in that case?  I'd imagine that it would
> automatically switch out of the powerdown config at wakeup before
> running software?  That doesn't seem like a great idea.

This is configurable. There is a bit that determines whether it should 
switch back to normal config automatically or not. What's interesting is 
that AFAIR current code uses automatic switch, which even with the glitch 
prevention in resume can glitch things, due to the period of time between 
wake-up and resume when pins are misconfigured.

I think we should just configure this bit for manual switch back and 
trigger the switch from .resumed() callback of the pin controller in 
pinctrl-s3c64xx driver. I will post a patch for this some day.

> I think that to make S3C64xx work properly we'd need to solve.
> 
> 
> I wouldn't be opposed to a re-spin to address some of the above, but I
> also won't object to this landing and remaining issues being addressed
> in future patches.  This patch definitely does make things better and
> no worse.  :)

Let's see. If nobody takes this until Monday, which is very likely, I will 
send new version.

> On exynos5250-snow (pinmux backported to 3.8):
> 
> Tested-by: Doug Anderson 
> 
> Reviewed-by: Doug Anderson 

Thanks.

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 3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 1:23 PM, Tomasz Figa  wrote:
>> I'd rather see you modify patch set #2 to provide some function to
>> retrieve just the eint mask and then use it here.  Your patch removes
>> this test and doesn't replace it with anything.  If you moved this
>> test to pinctrl directly you'd lose the test against intallow.
>
> Well, looking from the perspective of status before my patch, it just
> bypasses a test that is incorrect on DT-enabled platforms.

True.  What was there before was broken and this avoids the broken code.


> I agree that this test is rather reasonable to have, but it would require
> defining a new interface and moving all platforms to it, which for now
> would be a bit of overkill.

It seems like you could use the same type of solution as patch set #2?

...oh, but that's only for exynos!  I guess we would need something
similar for other platforms.  ...and until we do those other platforms
(including S3C64xx, I think) are still using the old
s3c_irqwake_eintmask, right?

...so overall your patch series only fully fixes exynos, though it
does make other platforms less broken which is a plus.  :)


> IMHO a separate series that sanitizes the whole PM handling in plat-
> samsung, including a rework of this check to make it cover all platforms
> in a generic and multiplatform-friendly way. What do you think?

Sure, I think that would be OK.


>> Ah, the important part here is the "saved", not the "save"!  The
>> "save" should get a NULL chip for all GPIOs and effectively be a
>> no-op.
>>
>> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
>> "saved" seems to transition things over to powerdown mode.  Hopefully
>> a future patch of yours adds that back for those platforms in the
>> pinmux world.  ...same for restore.
>
> S3C64xx can be switched to power down pin configuration manually, but if
> you don't do it, it will activate it automatically after entering sleep
> mode.

How would restore work in that case?  I'd imagine that it would
automatically switch out of the powerdown config at wakeup before
running software?  That doesn't seem like a great idea.

I think that to make S3C64xx work properly we'd need to solve.


I wouldn't be opposed to a re-spin to address some of the above, but I
also won't object to this landing and remaining issues being addressed
in future patches.  This patch definitely does make things better and
no worse.  :)

On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson 

Reviewed-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 4/6] pinctrl: samsung: Add support for SoC-specific suspend/resume callbacks

[Tomasz Figa]

On Friday 17 of May 2013 12:24:29 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> > +   if (ctrl->resume)
> > +   ctrl->resume(drvdata);
> > +
> 
> Having this resume at the beginning of the function seems right for
> restoring eint settings like you do in patch #6.
> 
> ...but if you need to try to handle the old s3c64xx and s5p64x0
> concept of "restored" to actually take GPIOs out of powerdown mode
> then you'll also need a callback at the end.
> 
> Does it make sense to add a second callback at the end of this function?

Right. I haven't thought of this. It might make sense to add .resumed() 
callback as well to handle this.

I guess it could be added as a part of patches for S3C64xx-specific 
pinctrl suspend/resume, that I will post some day.

> ...since it's unclear how we'll handle s3c64xx/s5p64x0 (or even if I'm
> misunderstanding and they're already handled somehow), I don't see any
> problems with this patch, so...
> 
> 
> On exynos5250-snow (pinmux backported to 3.8):
> 
> Tested-by: Doug Anderson 
> 
> Reviewed-by: Doug Anderson 

Thanks.

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 6/6] pinctrl: exynos: Handle suspend/resume of GPIO EINT registers

[Tomasz Figa]

On Friday 17 of May 2013 12:25:02 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> > Some GPIO EINT control registers needs to be preserved across
> > suspend/resume cycle. This patch extends the driver to take care of
> > this.
> 
> I was confused why we didn't seem to need this on exynos5250-snow but
> figured it out.  We only use interrupts on GPX lines which don't need
> this code.  ...but on any exynos5250 boards that use one of the other
> banks for interrupts you'd need it.  I tested by setting one of the
> registers related to GPA0 and found that this code is indeed needed on
> exynos5250.  :)
> 
> Just nits / optional comments below, so on exynos5250-snow (pinmux
> backported to 3.8):
> 
> Tested-by: Doug Anderson 
> 
> Reviewed-by: Doug Anderson 
> 
> > @@ -229,6 +235,11 @@ static int exynos_eint_gpio_init(struct
> > samsung_pinctrl_drv_data *d)> 
> > dev_err(dev, "gpio irq domain add failed\n");
> > return -ENXIO;
> > 
> > }
> > 
> > +
> > +   bank->soc_priv = devm_kzalloc(d->dev,
> > +   sizeof(struct exynos_eint_gpio_save),
> > GFP_KERNEL); +   if (!bank->soc_priv)
> > +   return -ENOMEM;
> 
> Slight nit to add this before the call to irq_domain_add_linear().
> demv() will handle freeing your memory but nothing will handle undoing
> the irq_domain_add_linear() if you return an error.

I'm a bit sceptical when it is about error handling in such cases. 
Basically if interrupt initialization fails, something is seriously wrong, 
either with your kernel config or with some code.

Since such case has been already unhandled in the driver (with nr_banks > 
1 = always), so I didn't bother to add any undoing here.

> > }
> > 
> > return 0;
> > 
> > @@ -528,6 +539,58 @@ static int exynos_eint_wkup_init(struct
> > samsung_pinctrl_drv_data *d)> 
> > return 0;
> >  
> >  }
> > 
> > +static void exynos_pinctrl_suspend_bank(
> > +   struct samsung_pinctrl_drv_data
> > *drvdata, +   struct samsung_pin_bank
> > *bank) +{
> > +   struct exynos_eint_gpio_save *save = bank->soc_priv;
> > +   void __iomem *regs = drvdata->virt_base;
> > +
> > +   save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET
> > +   + bank->eint_offset);
> > +   save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET
> > +   + 2 *
> > bank->eint_offset); +   save->eint_fltcon1 = readl(regs +
> > EXYNOS_GPIO_EFLTCON_OFFSET + 
> >  + 2 * bank->eint_offset + 4);
> Optional: I wish there were debug statements to help debug, like:
> 
> pr_debug("%s: save con %#010x\n", bank->name, save->eint_con);
> pr_debug("%s: save fltcon0 %#010x\n", bank->name, save->eint_fltcon0);
> pr_debug("%s: save fltcon1 %#010x\n", bank->name, save->eint_fltcon1);
> 

Right, seems reasonable.

> > +}
> > +
> > +static void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data
> > *drvdata) +{
> > +   struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
> > +   struct samsung_pin_bank *bank = ctrl->pin_banks;
> > +   int i;
> > +
> > +   for (i = 0; i < ctrl->nr_banks; ++i, ++bank)
> > +   if (bank->eint_type == EINT_TYPE_GPIO)
> > +   exynos_pinctrl_suspend_bank(drvdata, bank);
> > +}
> > +
> > +static void exynos_pinctrl_resume_bank(
> > +   struct samsung_pinctrl_drv_data
> > *drvdata, +   struct samsung_pin_bank
> > *bank) +{
> > +   struct exynos_eint_gpio_save *save = bank->soc_priv;
> > +   void __iomem *regs = drvdata->virt_base;
> > +
> 
> Optional: debug statements:
> 
> pr_debug("%s: con %#010x => %#010x\n", bank->name,
>   readl(regs + EXYNOS_GPIO_ECON_OFFSET + bank->eint_offset),
>   save->eint_con);
> pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name,
>   readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset),
>   save->eint_fltcon0);
> pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name,
>   readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset + 4),
>   save->eint_fltcon1);

OK. I wonder if this could be added in a separate patch or I should rather 
send v2 on Monday?

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 3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

[Tomasz Figa]

On Friday 17 of May 2013 12:24:04 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> > This patch makes legacy code on suspend/resume path being executed
> > conditionally, on non-DT platforms only, to fix suspend/resume of
> > DT-enabled systems, for which the code is inappropriate.
> > 
> > Signed-off-by: Tomasz Figa 
> > Signed-off-by: Kyungmin Park 
> > ---
> > 
> >  arch/arm/plat-samsung/pm.c | 17 -
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
> > index 53210ec..8ac2b2d 100644
> > --- a/arch/arm/plat-samsung/pm.c
> > +++ b/arch/arm/plat-samsung/pm.c
> > @@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
> > 
> >  * require a full power-cycle)
> > 
> > */
> > 
> > -   if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> > +   if (!of_have_populated_dt() &&
> > +   !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> 
> I'd rather see you modify patch set #2 to provide some function to
> retrieve just the eint mask and then use it here.  Your patch removes
> this test and doesn't replace it with anything.  If you moved this
> test to pinctrl directly you'd lose the test against intallow.

Well, looking from the perspective of status before my patch, it just 
bypasses a test that is incorrect on DT-enabled platforms.

I agree that this test is rather reasonable to have, but it would require 
defining a new interface and moving all platforms to it, which for now 
would be a bit of overkill.

IMHO a separate series that sanitizes the whole PM handling in plat-
samsung, including a rework of this check to make it cover all platforms 
in a generic and multiplatform-friendly way. What do you think?

> ...or do you think this test is no longer useful for some reason?
> 
> > !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow))
> > {
> > 
> > printk(KERN_ERR "%s: No wake-up sources!\n",
> > __func__);
> > printk(KERN_ERR "%s: Aborting sleep\n", __func__);
> > 
> > @@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
> > 
> > /* save all necessary core registers not covered by the
> > drivers */
> > 
> > -   samsung_pm_save_gpios();
> > -   samsung_pm_saved_gpios();
> > +   if (!of_have_populated_dt()) {
> > +   samsung_pm_save_gpios();
> > +   samsung_pm_saved_gpios();
> > +   }
> > +
> 
> Ah, the important part here is the "saved", not the "save"!  The
> "save" should get a NULL chip for all GPIOs and effectively be a
> no-op.
> 
> Skipping the "saved" is important of s3c64xx and s5p64x0 where the
> "saved" seems to transition things over to powerdown mode.  Hopefully
> a future patch of yours adds that back for those platforms in the
> pinmux world.  ...same for restore.

S3C64xx can be switched to power down pin configuration manually, but if 
you don't do it, it will activate it automatically after entering sleep 
mode.

> Summary: I've tested this on exynos5250-snow and it's reasonable but
> I'd love a response about the missing test before adding Reviewed-by.

Thanks.

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 2/6] ARM: EXYNOS: Fix EINT wake-up mask configuration when pinctrl is used

[Tomasz Figa]

Hi Doug,

On Friday 17 of May 2013 12:22:36 Doug Anderson wrote:
> Tomasz,
> 
> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> > On DT-enabled systems pinctrl-exynos driver is responsible for
> > handling
> > of wake-up EINT interrupts. This patch adjusts wake-up mask
> > configuration code to take wake-up mask value from pinctrl-exynos
> > driver on DT-enabled systems.
> > 
> > Signed-off-by: Tomasz Figa 
> > Signed-off-by: Kyungmin Park 
> > ---
> > 
> >  arch/arm/mach-exynos/include/mach/pm-core.h | 14 +-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> A little ugly

Hehe. I thought exactly the same, but I don't see any better solution at 
the moment, except persuading the whole suspend/resume support in plat-
samsung to be a friend of multiplatform, which will have to be done 
eventually, but at the moment I don't have time to work on it.

> because of the need to support old non-device tree
> boards, but seems reasonable.  Means that if you have a device tree
> you'd better be using pincontrol.  Assuming that's true now someone
> needs to go through and remove all of the device tree support (and
> bindings Documentation) for gpio-samsung.  Maybe someone has already
> started?

If I remember correctly Sylwester Nawrocki had some patches to remove 
that. He's on a leave right now, so he won't send them for a while, but I 
guess it's nothing urgent.

> On exynos5250-snow (pinmux backported to 3.8):
> 
> Tested-by: Doug Anderson 
> 
> Reviewed-by: Doug Anderson 

Thanks.

Best regards,
Tomasz

> On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> > On DT-enabled systems pinctrl-exynos driver is responsible for
> > handling
> > of wake-up EINT interrupts. This patch adjusts wake-up mask
> > configuration code to take wake-up mask value from pinctrl-exynos
> > driver on DT-enabled systems.
> > 
> > Signed-off-by: Tomasz Figa 
> > Signed-off-by: Kyungmin Park 
> > ---
> > 
> >  arch/arm/mach-exynos/include/mach/pm-core.h | 14 +-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h
> > b/arch/arm/mach-exynos/include/mach/pm-core.h index 7dbbfec..296090e
> > 100644
> > --- a/arch/arm/mach-exynos/include/mach/pm-core.h
> > +++ b/arch/arm/mach-exynos/include/mach/pm-core.h
> > @@ -18,8 +18,15 @@
> > 
> >  #ifndef __ASM_ARCH_PM_CORE_H
> >  #define __ASM_ARCH_PM_CORE_H __FILE__
> > 
> > +#include 
> > 
> >  #include 
> > 
> > +#ifdef CONFIG_PINCTRL_EXYNOS
> > +extern u32 exynos_get_eint_wake_mask(void);
> > +#else
> > +static inline u32 exynos_get_eint_wake_mask(void) { return
> > 0x; } +#endif
> > +
> > 
> >  static inline void s3c_pm_debug_init_uart(void)
> >  {
> >  
> > /* nothing here yet */
> > 
> > @@ -27,7 +34,12 @@ static inline void s3c_pm_debug_init_uart(void)
> > 
> >  static inline void s3c_pm_arch_prepare_irqs(void)
> >  {
> > 
> > -   __raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
> > +   u32 eintmask = s3c_irqwake_eintmask;
> > +
> > +   if (of_have_populated_dt())
> > +   eintmask = exynos_get_eint_wake_mask();
> > +
> > +   __raw_writel(eintmask, S5P_EINT_WAKEUP_MASK);
> > 
> > __raw_writel(s3c_irqwake_intmask & ~(1 << 31),
> > S5P_WAKEUP_MASK);
> >  
> >  }
> > 
> > --
> > 1.8.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in the body of a message to
> majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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 6/6] pinctrl: exynos: Handle suspend/resume of GPIO EINT registers

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> Some GPIO EINT control registers needs to be preserved across
> suspend/resume cycle. This patch extends the driver to take care of
> this.

I was confused why we didn't seem to need this on exynos5250-snow but
figured it out.  We only use interrupts on GPX lines which don't need
this code.  ...but on any exynos5250 boards that use one of the other
banks for interrupts you'd need it.  I tested by setting one of the
registers related to GPA0 and found that this code is indeed needed on
exynos5250.  :)

Just nits / optional comments below, so on exynos5250-snow (pinmux
backported to 3.8):

Tested-by: Doug Anderson 

Reviewed-by: Doug Anderson 


> @@ -229,6 +235,11 @@ static int exynos_eint_gpio_init(struct 
> samsung_pinctrl_drv_data *d)
> dev_err(dev, "gpio irq domain add failed\n");
> return -ENXIO;
> }
> +
> +   bank->soc_priv = devm_kzalloc(d->dev,
> +   sizeof(struct exynos_eint_gpio_save), GFP_KERNEL);
> +   if (!bank->soc_priv)
> +   return -ENOMEM;

Slight nit to add this before the call to irq_domain_add_linear().
demv() will handle freeing your memory but nothing will handle undoing
the irq_domain_add_linear() if you return an error.

> }
>
> return 0;
> @@ -528,6 +539,58 @@ static int exynos_eint_wkup_init(struct 
> samsung_pinctrl_drv_data *d)
> return 0;
>  }
>
> +static void exynos_pinctrl_suspend_bank(
> +   struct samsung_pinctrl_drv_data *drvdata,
> +   struct samsung_pin_bank *bank)
> +{
> +   struct exynos_eint_gpio_save *save = bank->soc_priv;
> +   void __iomem *regs = drvdata->virt_base;
> +
> +   save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET
> +   + bank->eint_offset);
> +   save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET
> +   + 2 * bank->eint_offset);
> +   save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET
> +   + 2 * bank->eint_offset + 4);

Optional: I wish there were debug statements to help debug, like:

pr_debug("%s: save con %#010x\n", bank->name, save->eint_con);
pr_debug("%s: save fltcon0 %#010x\n", bank->name, save->eint_fltcon0);
pr_debug("%s: save fltcon1 %#010x\n", bank->name, save->eint_fltcon1);


> +}
> +
> +static void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata)
> +{
> +   struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
> +   struct samsung_pin_bank *bank = ctrl->pin_banks;
> +   int i;
> +
> +   for (i = 0; i < ctrl->nr_banks; ++i, ++bank)
> +   if (bank->eint_type == EINT_TYPE_GPIO)
> +   exynos_pinctrl_suspend_bank(drvdata, bank);
> +}
> +
> +static void exynos_pinctrl_resume_bank(
> +   struct samsung_pinctrl_drv_data *drvdata,
> +   struct samsung_pin_bank *bank)
> +{
> +   struct exynos_eint_gpio_save *save = bank->soc_priv;
> +   void __iomem *regs = drvdata->virt_base;
> +

Optional: debug statements:

pr_debug("%s: con %#010x => %#010x\n", bank->name,
  readl(regs + EXYNOS_GPIO_ECON_OFFSET + bank->eint_offset),
  save->eint_con);
pr_debug("%s: fltcon0 %#010x => %#010x\n", bank->name,
  readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset),
  save->eint_fltcon0);
pr_debug("%s: fltcon1 %#010x => %#010x\n", bank->name,
  readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET + 2 * bank->eint_offset + 4),
  save->eint_fltcon1);

-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 5/6] pinctrl: samsung: Allow per-bank SoC-specific private data

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> This patch extends pin bank descriptor structure with SoC-specific
> private data field that allows SoC-specific drivers to store their own
> private data.
>
> Signed-off-by: Tomasz Figa 
> Signed-off-by: Kyungmin Park 
> ---
>  drivers/pinctrl/pinctrl-samsung.h | 1 +
>  1 file changed, 1 insertion(+)

Sure.  Seems like it could be squashed into the next patch, but I
think it looks fine separate too.

On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson 

Reviewed-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 4/6] pinctrl: samsung: Add support for SoC-specific suspend/resume callbacks

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> +   if (ctrl->resume)
> +   ctrl->resume(drvdata);
> +

Having this resume at the beginning of the function seems right for
restoring eint settings like you do in patch #6.

...but if you need to try to handle the old s3c64xx and s5p64x0
concept of "restored" to actually take GPIOs out of powerdown mode
then you'll also need a callback at the end.

Does it make sense to add a second callback at the end of this function?


...since it's unclear how we'll handle s3c64xx/s5p64x0 (or even if I'm
misunderstanding and they're already handled somehow), I don't see any
problems with this patch, so...


On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson 

Reviewed-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 3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> This patch makes legacy code on suspend/resume path being executed
> conditionally, on non-DT platforms only, to fix suspend/resume of
> DT-enabled systems, for which the code is inappropriate.
>
> Signed-off-by: Tomasz Figa 
> Signed-off-by: Kyungmin Park 
> ---
>  arch/arm/plat-samsung/pm.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
> index 53210ec..8ac2b2d 100644
> --- a/arch/arm/plat-samsung/pm.c
> +++ b/arch/arm/plat-samsung/pm.c
> @@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
>  * require a full power-cycle)
> */
>
> -   if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
> +   if (!of_have_populated_dt() &&
> +   !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&

I'd rather see you modify patch set #2 to provide some function to
retrieve just the eint mask and then use it here.  Your patch removes
this test and doesn't replace it with anything.  If you moved this
test to pinctrl directly you'd lose the test against intallow.

...or do you think this test is no longer useful for some reason?


> !any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow)) {
> printk(KERN_ERR "%s: No wake-up sources!\n", __func__);
> printk(KERN_ERR "%s: Aborting sleep\n", __func__);
> @@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
>
> /* save all necessary core registers not covered by the drivers */
>
> -   samsung_pm_save_gpios();
> -   samsung_pm_saved_gpios();
> +   if (!of_have_populated_dt()) {
> +   samsung_pm_save_gpios();
> +   samsung_pm_saved_gpios();
> +   }
> +

Ah, the important part here is the "saved", not the "save"!  The
"save" should get a NULL chip for all GPIOs and effectively be a
no-op.

Skipping the "saved" is important of s3c64xx and s5p64x0 where the
"saved" seems to transition things over to powerdown mode.  Hopefully
a future patch of yours adds that back for those platforms in the
pinmux world.  ...same for restore.


Summary: I've tested this on exynos5250-snow and it's reasonable but
I'd love a response about the missing test before adding Reviewed-by.

-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 2/6] ARM: EXYNOS: Fix EINT wake-up mask configuration when pinctrl is used

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> On DT-enabled systems pinctrl-exynos driver is responsible for handling
> of wake-up EINT interrupts. This patch adjusts wake-up mask
> configuration code to take wake-up mask value from pinctrl-exynos driver
> on DT-enabled systems.
>
> Signed-off-by: Tomasz Figa 
> Signed-off-by: Kyungmin Park 
> ---
>  arch/arm/mach-exynos/include/mach/pm-core.h | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)

A little ugly because of the need to support old non-device tree
boards, but seems reasonable.  Means that if you have a device tree
you'd better be using pincontrol.  Assuming that's true now someone
needs to go through and remove all of the device tree support (and
bindings Documentation) for gpio-samsung.  Maybe someone has already
started?


On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson 

Reviewed-by: Doug Anderson 

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> On DT-enabled systems pinctrl-exynos driver is responsible for handling
> of wake-up EINT interrupts. This patch adjusts wake-up mask
> configuration code to take wake-up mask value from pinctrl-exynos driver
> on DT-enabled systems.
>
> Signed-off-by: Tomasz Figa 
> Signed-off-by: Kyungmin Park 
> ---
>  arch/arm/mach-exynos/include/mach/pm-core.h | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h 
> b/arch/arm/mach-exynos/include/mach/pm-core.h
> index 7dbbfec..296090e 100644
> --- a/arch/arm/mach-exynos/include/mach/pm-core.h
> +++ b/arch/arm/mach-exynos/include/mach/pm-core.h
> @@ -18,8 +18,15 @@
>  #ifndef __ASM_ARCH_PM_CORE_H
>  #define __ASM_ARCH_PM_CORE_H __FILE__
>
> +#include 
>  #include 
>
> +#ifdef CONFIG_PINCTRL_EXYNOS
> +extern u32 exynos_get_eint_wake_mask(void);
> +#else
> +static inline u32 exynos_get_eint_wake_mask(void) { return 0x; }
> +#endif
> +
>  static inline void s3c_pm_debug_init_uart(void)
>  {
> /* nothing here yet */
> @@ -27,7 +34,12 @@ static inline void s3c_pm_debug_init_uart(void)
>
>  static inline void s3c_pm_arch_prepare_irqs(void)
>  {
> -   __raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
> +   u32 eintmask = s3c_irqwake_eintmask;
> +
> +   if (of_have_populated_dt())
> +   eintmask = exynos_get_eint_wake_mask();
> +
> +   __raw_writel(eintmask, S5P_EINT_WAKEUP_MASK);
> __raw_writel(s3c_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
>  }
>
> --
> 1.8.2.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/6] pinctrl: exynos: Add support for set_irq_wake of wake-up EINTs

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 9:24 AM, Tomasz Figa  wrote:
> This patch adds support of IRQ wake-up ability configuration for
> wake-up EINTs on Exynos SoCs.
>
> Signed-off-by: Tomasz Figa 
> Signed-off-by: Kyungmin Park 
> ---
>  drivers/pinctrl/pinctrl-exynos.c | 23 +++
>  1 file changed, 23 insertions(+)

On exynos5250-snow (pinmux backported to 3.8):

Tested-by: Doug Anderson 

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

[PATCH 6/6] pinctrl: exynos: Handle suspend/resume of GPIO EINT registers

[Tomasz Figa]

Some GPIO EINT control registers needs to be preserved across
suspend/resume cycle. This patch extends the driver to take care of
this.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 drivers/pinctrl/pinctrl-exynos.c | 83 
 drivers/pinctrl/pinctrl-exynos.h |  1 +
 2 files changed, 84 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 4f868e5..908d24a 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -196,6 +196,12 @@ static irqreturn_t exynos_eint_gpio_irq(int irq, void 
*data)
return IRQ_HANDLED;
 }
 
+struct exynos_eint_gpio_save {
+   u32 eint_con;
+   u32 eint_fltcon0;
+   u32 eint_fltcon1;
+};
+
 /*
  * exynos_eint_gpio_init() - setup handling of external gpio interrupts.
  * @d: driver data of samsung pinctrl driver.
@@ -229,6 +235,11 @@ static int exynos_eint_gpio_init(struct 
samsung_pinctrl_drv_data *d)
dev_err(dev, "gpio irq domain add failed\n");
return -ENXIO;
}
+
+   bank->soc_priv = devm_kzalloc(d->dev,
+   sizeof(struct exynos_eint_gpio_save), GFP_KERNEL);
+   if (!bank->soc_priv)
+   return -ENOMEM;
}
 
return 0;
@@ -528,6 +539,58 @@ static int exynos_eint_wkup_init(struct 
samsung_pinctrl_drv_data *d)
return 0;
 }
 
+static void exynos_pinctrl_suspend_bank(
+   struct samsung_pinctrl_drv_data *drvdata,
+   struct samsung_pin_bank *bank)
+{
+   struct exynos_eint_gpio_save *save = bank->soc_priv;
+   void __iomem *regs = drvdata->virt_base;
+
+   save->eint_con = readl(regs + EXYNOS_GPIO_ECON_OFFSET
+   + bank->eint_offset);
+   save->eint_fltcon0 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET
+   + 2 * bank->eint_offset);
+   save->eint_fltcon1 = readl(regs + EXYNOS_GPIO_EFLTCON_OFFSET
+   + 2 * bank->eint_offset + 4);
+}
+
+static void exynos_pinctrl_suspend(struct samsung_pinctrl_drv_data *drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
+   struct samsung_pin_bank *bank = ctrl->pin_banks;
+   int i;
+
+   for (i = 0; i < ctrl->nr_banks; ++i, ++bank)
+   if (bank->eint_type == EINT_TYPE_GPIO)
+   exynos_pinctrl_suspend_bank(drvdata, bank);
+}
+
+static void exynos_pinctrl_resume_bank(
+   struct samsung_pinctrl_drv_data *drvdata,
+   struct samsung_pin_bank *bank)
+{
+   struct exynos_eint_gpio_save *save = bank->soc_priv;
+   void __iomem *regs = drvdata->virt_base;
+
+   writel(save->eint_con, regs + EXYNOS_GPIO_ECON_OFFSET
+   + bank->eint_offset);
+   writel(save->eint_fltcon0, regs + EXYNOS_GPIO_EFLTCON_OFFSET
+   + 2 * bank->eint_offset);
+   writel(save->eint_fltcon1, regs + EXYNOS_GPIO_EFLTCON_OFFSET
+   + 2 * bank->eint_offset + 4);
+}
+
+static void exynos_pinctrl_resume(struct samsung_pinctrl_drv_data *drvdata)
+{
+   struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
+   struct samsung_pin_bank *bank = ctrl->pin_banks;
+   int i;
+
+   for (i = 0; i < ctrl->nr_banks; ++i, ++bank)
+   if (bank->eint_type == EINT_TYPE_GPIO)
+   exynos_pinctrl_resume_bank(drvdata, bank);
+}
+
 /* pin banks of exynos4210 pin-controller 0 */
 static struct samsung_pin_bank exynos4210_pin_banks0[] = {
EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpa0", 0x00),
@@ -591,6 +654,8 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = {
.geint_pend = EXYNOS_GPIO_EPEND_OFFSET,
.svc= EXYNOS_SVC_OFFSET,
.eint_gpio_init = exynos_eint_gpio_init,
+   .suspend= exynos_pinctrl_suspend,
+   .resume = exynos_pinctrl_resume,
.label  = "exynos4210-gpio-ctrl0",
}, {
/* pin-controller instance 1 data */
@@ -605,6 +670,8 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = {
.svc= EXYNOS_SVC_OFFSET,
.eint_gpio_init = exynos_eint_gpio_init,
.eint_wkup_init = exynos_eint_wkup_init,
+   .suspend= exynos_pinctrl_suspend,
+   .resume = exynos_pinctrl_resume,
.label  = "exynos4210-gpio-ctrl1",
}, {
/* pin-controller instance 2 data */
@@ -686,6 +753,8 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = {
.geint_pend = EXYNOS_GPIO_EPEND_OFFSET,
.svc= EXYNOS_SVC_OFFSET,
   

[PATCH 4/6] pinctrl: samsung: Add support for SoC-specific suspend/resume callbacks

[Tomasz Figa]

SoC-specific driver might require additional save and restore of
registers. This patch adds pair of SoC-specific callbacks per pinctrl
device to account for this.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 drivers/pinctrl/pinctrl-samsung.c | 6 ++
 drivers/pinctrl/pinctrl-samsung.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.c 
b/drivers/pinctrl/pinctrl-samsung.c
index 15db258..63ac22e 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -1010,6 +1010,9 @@ static void samsung_pinctrl_suspend_dev(
 reg, bank->pm_save[PINCFG_TYPE_FUNC]);
}
}
+
+   if (ctrl->suspend)
+   ctrl->suspend(drvdata);
 }
 
 /**
@@ -1026,6 +1029,9 @@ static void samsung_pinctrl_resume_dev(struct 
samsung_pinctrl_drv_data *drvdata)
void __iomem *virt_base = drvdata->virt_base;
int i;
 
+   if (ctrl->resume)
+   ctrl->resume(drvdata);
+
for (i = 0; i < ctrl->nr_banks; i++) {
struct samsung_pin_bank *bank = &ctrl->pin_banks[i];
void __iomem *reg = virt_base + bank->pctl_offset;
diff --git a/drivers/pinctrl/pinctrl-samsung.h 
b/drivers/pinctrl/pinctrl-samsung.h
index 9f5cc81..b316d9f 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -187,6 +187,9 @@ struct samsung_pin_ctrl {
 
int (*eint_gpio_init)(struct samsung_pinctrl_drv_data *);
int (*eint_wkup_init)(struct samsung_pinctrl_drv_data *);
+   void(*suspend)(struct samsung_pinctrl_drv_data *);
+   void(*resume)(struct samsung_pinctrl_drv_data *);
+
char*label;
 };
 
-- 
1.8.2.1

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

[PATCH 5/6] pinctrl: samsung: Allow per-bank SoC-specific private data

[Tomasz Figa]

This patch extends pin bank descriptor structure with SoC-specific
private data field that allows SoC-specific drivers to store their own
private data.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 drivers/pinctrl/pinctrl-samsung.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pinctrl/pinctrl-samsung.h 
b/drivers/pinctrl/pinctrl-samsung.h
index b316d9f..26d3519 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -139,6 +139,7 @@ struct samsung_pin_bank {
u32 eint_mask;
u32 eint_offset;
char*name;
+   void*soc_priv;
struct device_node *of_node;
struct samsung_pinctrl_drv_data *drvdata;
struct irq_domain *irq_domain;
-- 
1.8.2.1

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

[PATCH 3/6] ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms

[Tomasz Figa]

This patch makes legacy code on suspend/resume path being executed
conditionally, on non-DT platforms only, to fix suspend/resume of
DT-enabled systems, for which the code is inappropriate.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 arch/arm/plat-samsung/pm.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
index 53210ec..8ac2b2d 100644
--- a/arch/arm/plat-samsung/pm.c
+++ b/arch/arm/plat-samsung/pm.c
@@ -261,7 +261,8 @@ static int s3c_pm_enter(suspend_state_t state)
 * require a full power-cycle)
*/
 
-   if (!any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
+   if (!of_have_populated_dt() &&
+   !any_allowed(s3c_irqwake_intmask, s3c_irqwake_intallow) &&
!any_allowed(s3c_irqwake_eintmask, s3c_irqwake_eintallow)) {
printk(KERN_ERR "%s: No wake-up sources!\n", __func__);
printk(KERN_ERR "%s: Aborting sleep\n", __func__);
@@ -270,8 +271,11 @@ static int s3c_pm_enter(suspend_state_t state)
 
/* save all necessary core registers not covered by the drivers */
 
-   samsung_pm_save_gpios();
-   samsung_pm_saved_gpios();
+   if (!of_have_populated_dt()) {
+   samsung_pm_save_gpios();
+   samsung_pm_saved_gpios();
+   }
+
s3c_pm_save_uarts();
s3c_pm_save_core();
 
@@ -310,8 +314,11 @@ static int s3c_pm_enter(suspend_state_t state)
 
s3c_pm_restore_core();
s3c_pm_restore_uarts();
-   samsung_pm_restore_gpios();
-   s3c_pm_restored_gpios();
+
+   if (!of_have_populated_dt()) {
+   samsung_pm_restore_gpios();
+   s3c_pm_restored_gpios();
+   }
 
s3c_pm_debug_init();
 
-- 
1.8.2.1

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

[PATCH 1/6] pinctrl: exynos: Add support for set_irq_wake of wake-up EINTs

[Tomasz Figa]

This patch adds support of IRQ wake-up ability configuration for
wake-up EINTs on Exynos SoCs.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 drivers/pinctrl/pinctrl-exynos.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index ac74281..4f868e5 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -326,6 +326,28 @@ static int exynos_wkup_irq_set_type(struct irq_data *irqd, 
unsigned int type)
return 0;
 }
 
+static u32 exynos_eint_wake_mask = 0x;
+
+u32 exynos_get_eint_wake_mask(void)
+{
+   return exynos_eint_wake_mask;
+}
+
+static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)
+{
+   struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+   unsigned long bit = 1UL << (2 * bank->eint_offset + irqd->hwirq);
+
+   pr_info("wake %s for irq %d\n", on ? "enabled" : "disabled", irqd->irq);
+
+   if (!on)
+   exynos_eint_wake_mask |= bit;
+   else
+   exynos_eint_wake_mask &= ~bit;
+
+   return 0;
+}
+
 /*
  * irq_chip for wakeup interrupts
  */
@@ -335,6 +357,7 @@ static struct irq_chip exynos_wkup_irq_chip = {
.irq_mask   = exynos_wkup_irq_mask,
.irq_ack= exynos_wkup_irq_ack,
.irq_set_type   = exynos_wkup_irq_set_type,
+   .irq_set_wake   = exynos_wkup_irq_set_wake,
 };
 
 /* interrupt handler for wakeup interrupts 0..15 */
-- 
1.8.2.1

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

[PATCH 2/6] ARM: EXYNOS: Fix EINT wake-up mask configuration when pinctrl is used

[Tomasz Figa]

On DT-enabled systems pinctrl-exynos driver is responsible for handling
of wake-up EINT interrupts. This patch adjusts wake-up mask
configuration code to take wake-up mask value from pinctrl-exynos driver
on DT-enabled systems.

Signed-off-by: Tomasz Figa 
Signed-off-by: Kyungmin Park 
---
 arch/arm/mach-exynos/include/mach/pm-core.h | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-exynos/include/mach/pm-core.h 
b/arch/arm/mach-exynos/include/mach/pm-core.h
index 7dbbfec..296090e 100644
--- a/arch/arm/mach-exynos/include/mach/pm-core.h
+++ b/arch/arm/mach-exynos/include/mach/pm-core.h
@@ -18,8 +18,15 @@
 #ifndef __ASM_ARCH_PM_CORE_H
 #define __ASM_ARCH_PM_CORE_H __FILE__
 
+#include 
 #include 
 
+#ifdef CONFIG_PINCTRL_EXYNOS
+extern u32 exynos_get_eint_wake_mask(void);
+#else
+static inline u32 exynos_get_eint_wake_mask(void) { return 0x; }
+#endif
+
 static inline void s3c_pm_debug_init_uart(void)
 {
/* nothing here yet */
@@ -27,7 +34,12 @@ static inline void s3c_pm_debug_init_uart(void)
 
 static inline void s3c_pm_arch_prepare_irqs(void)
 {
-   __raw_writel(s3c_irqwake_eintmask, S5P_EINT_WAKEUP_MASK);
+   u32 eintmask = s3c_irqwake_eintmask;
+
+   if (of_have_populated_dt())
+   eintmask = exynos_get_eint_wake_mask();
+
+   __raw_writel(eintmask, S5P_EINT_WAKEUP_MASK);
__raw_writel(s3c_irqwake_intmask & ~(1 << 31), S5P_WAKEUP_MASK);
 }
 
-- 
1.8.2.1

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

[PATCH 0/6] Fix suspend/resume issues created by pinmux on exynos, part 2

[Tomasz Figa]

This series aims at fixing problems with suspend/resume on Exynos SoCs
introduced by migration of GPIO and EINT support to pin control driver
on DT-enabled platforms.

The patches fix following issues:
 - missing support for IRQ wake of pinctrl-exynos driver
 - legacy GPIO and EINT save/restore handling in platform PM code
   inappropriate on pinctrl-enabled platforms
 - several EINT-related registers must be saved as well
 
The series is based on a patch by Doug Anderson that adds suspend/restore
of pinctrl registers to pinctrl-samsung driver:
[PATCH v3] pinctrl: samsung: fix suspend/resume functionality
http://www.spinics.net/lists/kernel/msg1534119.html

On Exynos4210-based Trats board:
Tested-by: Tomasz Figa 

Tomasz Figa (6):
  pinctrl: exynos: Add support for set_irq_wake of wake-up EINTs
  ARM: EXYNOS: Fix EINT wake-up mask configuration when pinctrl is used
  ARM: SAMSUNG: pm: Adjust for pinctrl- and DT-enabled platforms
  pinctrl: samsung: Add support for SoC-specific suspend/resume
callbacks
  pinctrl: samsung: Allow per-bank SoC-specific private data
  pinctrl: exynos: Handle suspend/resume of GPIO EINT registers

 arch/arm/mach-exynos/include/mach/pm-core.h |  14 +++-
 arch/arm/plat-samsung/pm.c  |  17 +++--
 drivers/pinctrl/pinctrl-exynos.c| 106 
 drivers/pinctrl/pinctrl-exynos.h|   1 +
 drivers/pinctrl/pinctrl-samsung.c   |   6 ++
 drivers/pinctrl/pinctrl-samsung.h   |   4 ++
 6 files changed, 142 insertions(+), 6 deletions(-)

-- 
1.8.2.1

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

Re: Pulls and drive strengths in the pinctrl world

[Doug Anderson]

Tomasz,

On Fri, May 17, 2013 at 5:38 AM, Tomasz Figa  wrote:
>> You might want to have a debugfs file in your driver for inspecting
>> them though, that sounds like it could be helpful. I'd recommend
>> augmenting your .pin_config_dbg_show() callback in the
>> struct pinconf_ops to display this for each pin, in addition to the
>> current configuration.
>
> Seems reasonable.

If you do this I'd love to be CCed.  I have a super-ugly userspace
tool that shows this info and have been contemplating moving it into
debugfs and cleaning it up there.

See 

A small snippet of the output looks like:

GPIO x3[4]@0x11400c60: con=0x0 dat=0 pulldwn(1) drv=1(0)
GPIO x3[5]@0x11400c60: con=0xf dat=1  nopull(0) drv=1(0)
GPIO x3[6]@0x11400c60: con=0x0 dat=1 pulldwn(1) drv=1(0)
GPIO x3[7]@0x11400c60: con=0x0 dat=0 pulldwn(1) drv=1(0)
GPIO e0[0]@0x1340: con=0x0 dat=0 pulldwn(1) drv=1(0) pdn= low(0),
pdn= nopull(0)
GPIO e0[1]@0x1340: con=0x0 dat=0 pulldwn(1) drv=1(0) pdn=  in(2),
pdn=pulldwn(1)
GPIO e0[2]@0x1340: con=0x0 dat=0 pulldwn(1) drv=1(0) pdn=  in(2),
pdn=pulldwn(1)

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

Re: [PATCH v3] pinctrl: samsung: fix suspend/resume functionality

[Tomasz Figa]

Hi Doug,

On Thursday 16 of May 2013 21:33:18 Doug Anderson wrote:
> The GPIO states need to be restored after s2r and this is not currently
> supported in the pinctrl driver. This patch saves the gpio states before
> suspend and restores them after resume.
> 
> Saving and restoring is done very early using syscore_ops and must
> happen before pins are released from their powerdown state.
> 
> Patch originally from Prathyush K  but
> rewritten by Doug Anderson .
> 
> Signed-off-by: Prathyush K 
> Signed-off-by: Doug Anderson 
> ---
> Changes in v3:
> - Skip save and restore for banks with no powerdown config.
> 
> Changes in v2:
> - Now uses sycore_ops to make sure we're early enough.
> - Try to handle two CON registers better.
> - Should handle s3c24xx better as per Heiko.
> - Simpler code; no longer tries to avoid glitching lines since
>   we _think_ all current SoCs should have pins in power down state
>   when the restore is called.
> - Dropped eint patch for now; Tomasz will post his version.
> 
>  drivers/pinctrl/pinctrl-samsung.c | 148
> ++ drivers/pinctrl/pinctrl-samsung.h | 
>  5 ++
>  2 files changed, 153 insertions(+)

On Exynos4210-based Trats board:

Tested-by: Tomasz Figa 

Acked-by: Tomasz Figa 

I will send several complementary patches in a while.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

> diff --git a/drivers/pinctrl/pinctrl-samsung.c
> b/drivers/pinctrl/pinctrl-samsung.c index 9763668..d45e36f 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "core.h"
>  #include "pinctrl-samsung.h"
> @@ -48,6 +49,9 @@ static struct pin_config {
>   { "samsung,pin-pud-pdn", PINCFG_TYPE_PUD_PDN },
>  };
> 
> +/* Global list of devices (struct samsung_pinctrl_drv_data) */
> +LIST_HEAD(drvdata_list);
> +
>  static unsigned int pin_base;
> 
>  static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
> @@ -961,9 +965,145 @@ static int samsung_pinctrl_probe(struct
> platform_device *pdev) ctrl->eint_wkup_init(drvdata);
> 
>   platform_set_drvdata(pdev, drvdata);
> +
> + /* Add to the global list */
> + list_add_tail(&drvdata->node, &drvdata_list);
> +
>   return 0;
>  }
> 
> +#ifdef CONFIG_PM
> +
> +/**
> + * samsung_pinctrl_suspend_dev - save pinctrl state for suspend for a
> device + *
> + * Save data for all banks handled by this device.
> + */
> +static void samsung_pinctrl_suspend_dev(
> + struct samsung_pinctrl_drv_data *drvdata)
> +{
> + struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
> + void __iomem *virt_base = drvdata->virt_base;
> + int i;
> +
> + for (i = 0; i < ctrl->nr_banks; i++) {
> + struct samsung_pin_bank *bank = &ctrl->pin_banks[i];
> + void __iomem *reg = virt_base + bank->pctl_offset;
> +
> + u8 *offs = bank->type->reg_offset;
> + u8 *widths = bank->type->fld_width;
> + enum pincfg_type type;
> +
> + /* Registers without a powerdown config aren't lost */
> + if (!widths[PINCFG_TYPE_CON_PDN])
> + continue;
> +
> + for (type = 0; type < PINCFG_TYPE_NUM; type++)
> + if (widths[type])
> + bank->pm_save[type] = readl(reg + offs[type]);
> +
> + if (widths[PINCFG_TYPE_FUNC] * bank->nr_pins > 32) {
> + /* Some banks have two config registers */
> + bank->pm_save[PINCFG_TYPE_NUM] =
> + readl(reg + offs[PINCFG_TYPE_FUNC] + 4);
> + pr_debug("Save %s @ %p (con %#010x %08x)\n",
> +  bank->name, reg,
> +  bank->pm_save[PINCFG_TYPE_FUNC],
> +  bank->pm_save[PINCFG_TYPE_NUM]);
> + } else {
> + pr_debug("Save %s @ %p (con %#010x)\n", bank->name,
> +  reg, bank->pm_save[PINCFG_TYPE_FUNC]);
> + }
> + }
> +}
> +
> +/**
> + * samsung_pinctrl_resume_dev - restore pinctrl state from suspend for a
> device + *
> + * Restore one of the banks that was saved during suspend.
> + *
> + * We don't bother doing anything complicated to avoid glitching lines
> since + * we're called before pad retention is turned off.
> + */
> +static void samsung_pinctrl_resume_dev(struct samsung_pinctrl_drv_data
> *drvdata) +{
> + struct samsung_pin_ctrl *ctrl = drvdata->ctrl;
> + void __iomem *virt_base = drvdata->virt_base;
> + int i;
> +
> + for (i = 0; i < ctrl->nr_banks; i++) {
> + struct samsung_pin_bank *bank = &ctrl->pin_banks[i];
> + void __iomem *reg = virt_base + bank->pctl_offset;
> +
> + u8 *offs = bank->type->reg_offset;
> + u8 *widths = bank->type->fld_width;
> 

Re: Pulls and drive strengths in the pinctrl world

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:22 Wed 15 May , Stephen Warren wrote:
> On 05/15/2013 06:13 PM, Tomasz Figa wrote:
> > On Wednesday 15 of May 2013 16:55:37 Doug Anderson wrote:
> >> Tomasz / Linus,
> >>
> >> On Wed, May 15, 2013 at 3:06 PM, Tomasz Figa  
> > wrote:
> >>> Yes. I don't like the current way too much either, duplication being
> >>> one of the reasons.
> >>
> >> Do you have any other ideas?  It sounds like Linus didn't like my
> >> suggestion and makes some good points...
> > 
> > I don't have anything interesting at the moment. It's a bit late now here 
> > (2 AM), so I'm going to get some sleep first.
> > 
> > Also after reading Stephen's reply, I'm wondering if hogging wouldn't 
> > solve the problem indeed. (It might have to be fixed on pinctrl-samsung 
> > first, as last time I tried to use it, it caused some errors from pinctrl 
> > core, but haven't time to track them down, as it wasn't anything important 
> > at that time).
> 
> One issue I noticed with the DT fragments earlier in this thread. It
> looks like hogs in the Samsung pinctrl bingings end up looking like:
> 
> pinctrl {
> pina {
> samsung,pins = ;
> samsung,pin-function = <0xf>;
> samsung,pin-pud = <0>;
> ...

I have a huge issue here that we had on at91 too

we are going to have a huge numbet of node

and on at91 we handle the pin the same way as samsung
and ST have also a similiar IP

so I'll prefer to reuse the AT91 DT bindings

as said by Linus I just push a cleanup of the magic by using Macro
which make it really readable now

some extract of the sama5 pinctrl

mmc0 {
pinctrl_mmc0_clk_cmd_dat0: mmc0_clk_cmd_dat0 {
atmel,pins =
;   /* PD1 periph A MCI0_DA0 with pullup */
};
pinctrl_mmc0_dat1_3: mmc0_dat1_3 {
atmel,pins =
;   /* PD4 periph A MCI0_DA3 with pullup */
};
pinctrl_mmc0_dat4_7: mmc0_dat4_7 {
atmel,pins =
;   /* PD8 periph A MCI0_DA7 with pullup, conflicts 
with PWML3 */
};
};

of sam9g45

i2c_gpio2 {
pinctrl_i2c_gpio2: i2c_gpio2-0 {
atmel,pins =
;/* PB5 gpio multidrive I2C2 clock */
};
};

so we could share the c code too

Best Regards,
J,
> };
> pinp {
> samsung,pins = ;
> samsung,pin-function = <0xe>;
> samsung,pin-pud = <1>;
> ...
> };
> pinx {
> samsung,pins = ;
> samsung,pin-function = <0xd>;
> samsung,pin-pud = <2>;
> ...
> };
> 
> pinctrl-names = "default";
> pinctrl-0 = <&pina &pinp &pinx>;
> };
> 
> That pinctrl-0 property could get rather large (hard to write/maintain,
> unwieldy) if it needs to set up lots of different configurations. That's
> why I made the equivalent Tegra bindings be:
> 
> pinctrl {
> pins_default {
> pina {
> samsung,pins = ;
> samsung,pin-function = <0xf>;
> samsung,pin-pud = <0>;
> ...
> };
> pinp {
> samsung,pins = ;
> samsung,pin-function = <0xe>;
> samsung,pin-pud = <1>;
> ...
> };
> pinx {
> samsung,pins = ;
> samsung,pin-function = <0xd>;
> samsung,pin-pud = <2>;
> ...
> };
> };
> 
> pinctrl-names = "default";
> pinctrl-0 = <&pins_default>;
> };
> 
> The extra level within the "pinctrl configuration node" ("pins_default"
> here) makes the pinctrl-0 property a lot easier to write, and the
> advantage happens at every use-site that needs to configure different
> subsets of the relevant pins in different ways.
> 
> If you're changing all the bindings anyway, introducing this extra level
> might be something to think about.
> 
> I did try to explain my philosophy here when we all got together to
> design the pinctrl bindings, but I obviously didn't explain it well
> enough, or people didn't like it anyway.
> ___
> devicetree-discuss mailing list
> devicetree-disc...@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
--
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: Pulls and drive strengths in the pinctrl world

[Tomasz Figa]

Hi Linus,

On Friday 17 of May 2013 13:59:54 Linus Walleij wrote:
> On Fri, May 17, 2013 at 11:09 AM, Tomasz Figa  wrote:
> >> Just add another state, pctldev->hog_shutdown to this, and
> >> add an operation pinctrl_force_poweroff() in the same spirit as
> >> pinctrl_force_sleep() that we already have.
> >> 
> >> Add a new state to include/linux/pinctrl/pinctrl-state.h:
> >> #define PINCTRL_STATE_POWEROFF "poweroff"
> >> 
> >> And define you pin table to hog these pins with the mentioned
> >> default and poweroff states.
> >> 
> >> Result: pinctrl core keeps track of your offstate too.
> > 
> > Power down mode settings on our pin controller are completely unrelated to
> > normal mode settings. You can set them once in appropriate registers and
> > pins are switched to them automatically when the SoC enters sleep mode.
> 
> Aha so it's actually automatic sleep modes, not power down
> (as in, disconnect the power and then push the "on" button to
> get it back up).
> 
> Please remember to document it per above in the code and the
> device tree, so everybody understands what it is.

Sure.

> > So IMHO in our case power mode settings are just additional pin
> > configuration options, next to pull-up/-down and driver strength.
> 
> I see. Yes that is different.
> 
> You might want to have a debugfs file in your driver for inspecting
> them though, that sounds like it could be helpful. I'd recommend
> augmenting your .pin_config_dbg_show() callback in the
> struct pinconf_ops to display this for each pin, in addition to the
> current configuration.

Seems reasonable.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

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

Re: [PATCH 1/2] video: exynos_dp: Add parsing of gpios pins to exynos-dp driver

[Tomasz Figa]

Hi Jingoo,

On Thursday 16 of May 2013 02:03:59 한진구 wrote:
> Tuesday, May 14, 2013 11:17 PM, Vikas Sajjan wrote:
> 
> > 
> > Hi Vikas,
> > 
> > On Tuesday 14 of May 2013 18:25:51 Vikas Sajjan wrote:
> > 
> > >  Adds GPIO parsing functionality for "LCD backlight" and "LCD enable"
> > >  GPIO pins of exynos dp controller.
> > >
> > >
> > >
> > > Signed-off-by: Vikas Sajjan 
> > > ---
> > > 
> > >  drivers/video/exynos/exynos_dp_core.c |   45
> > > 
> > > + 1 file changed, 45 insertions(+)
> > >
> > >
> > 
> > 
> > I don't think that Exynos DP driver is right place for such code.
> > Backlight
 and LCD drivers are responsible for backlight and LCD power
> > control using backlight and LCD subsystems.
> > 
> > IMHO the correct solution would be to either extend existing
> > backlight/lcd
> > drivers found in drivers/video/backlight to support direct GPIO control
> > and
 parse GPIO pins from device tree or create new gpio_bl and gpio_lcd
> > drivers.
> 
> Hi Vikas Sajian,
> 
> I agree with Tomasz Figa's opinion.
> Backlight/LCD framework should be used.
> eDP panel backlight on SMDK5210 board can be controlled by PWM;
> thus, pwm-backlight driver should be used.
> Also, eDP panel reset pin should be controlled by using
> platform-lcd driver.
> 
> 
> > 
> > CCing Richard, Florian and linux-fbdev.
> 
> 
> Also, I have been doing backlight reviews instead of Richard,
> please do CC'ing me.

OK. I used get_maintainers script, but it seems like the result was a bit off 
in this case. Will remember for future.

Best regards,
-- 
Tomasz Figa
Linux Kernel Developer
Samsung R&D Institute Poland
Samsung Electronics

--
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: [RFC 2/4] dma: add dmaengine driver for Samsung s3c24xx SoCs

[Linus Walleij]

On Wed, May 15, 2013 at 10:31 PM, Heiko Stübner  wrote:

> If I understand the writel semantics and the thread from you from 2011 [0]
> correctly, only the writel to DMASKTRIG mustn't be relaxed to make sure the
> settings registers are written to before, so like:
>
>writel_relaxed(txd->src_addr, phy->base + DISRC);
>writel_relaxed(txd->disrcc, phy->base + DISRCC);
>writel_relaxed(txd->dst_addr, phy->base + DIDST);
>writel_relaxed(txd->didstc, phy->base + DIDSTC);
>writel_relaxed(dcon, phy->base + DCON);
>
>val = readl_relaxed(phy->base + DMASKTRIG);
>val &= ~DMASKTRIG_STOP;
>val |= DMASKTRIG_ON;
>writel(val, phy->base + DMASKTRIG);

Yep. That will drain write buffers etc and make sure all outstanding writes hit
the hardware.

Yours,
Linus Walleij
--
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 16/30] thermal: exynos: Make the zone handling dependent on trip count

[jonghwa3 . lee]

On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:

> This code simplifies the zone handling to use the trip count passed
> by the TMU driver. This also helps in adding more zone support.
> 
> Acked-by: Kukjin Kim 
> Signed-off-by: Amit Daniel Kachhap 
> ---
>  drivers/thermal/samsung/exynos_thermal_common.c |   55 
> ---
>  drivers/thermal/samsung/exynos_thermal_common.h |2 -
>  2 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_thermal_common.c 
> b/drivers/thermal/samsung/exynos_thermal_common.c
> index 86d39aa..2369417 100644
> --- a/drivers/thermal/samsung/exynos_thermal_common.c
> +++ b/drivers/thermal/samsung/exynos_thermal_common.c
> @@ -78,17 +78,16 @@ static int exynos_set_mode(struct thermal_zone_device 
> *thermal,
>  static int exynos_get_trip_type(struct thermal_zone_device *thermal, int 
> trip,
>enum thermal_trip_type *type)
>  {
> - switch (GET_ZONE(trip)) {
> - case MONITOR_ZONE:
> - case WARN_ZONE:
> - *type = THERMAL_TRIP_ACTIVE;
> - break;
> - case PANIC_ZONE:
> - *type = THERMAL_TRIP_CRITICAL;
> - break;
> - default:
> + struct exynos_thermal_zone *th_zone = thermal->devdata;
> + int max_trip = th_zone->sensor_conf->trip_data.trip_count;
> +
> + if (trip < 0 || trip >= max_trip)
>   return -EINVAL;
> - }
> + else if (trip == (max_trip - 1))
> + *type = THERMAL_TRIP_CRITICAL;
> + else
> + *type = THERMAL_TRIP_ACTIVE;
> +


 In current exynos_thermal driver, it is hard to set various trip type for each
trip, especially passive type. (not impossible, but complicated)

What do you think we just keep trip information with trip temperature in private
data? I mean if we just make trip level information to driver's private data
(like exynos_thermal_zone), it might be helpful to control trip type and trip
temperature. Like this,

struct exynos_thermal_trip_info {
int trip_temp;
enum thermal_trip_type type;
};
struct exynos_thermal_device {  
.
struct exynos_thermal_trip_info *trips;
int num_trips;
};

Thanks,
Jonghwa

>   return 0;
>  }
>  
> @@ -97,8 +96,9 @@ static int exynos_get_trip_temp(struct thermal_zone_device 
> *thermal, int trip,
>   unsigned long *temp)
>  {
>   struct exynos_thermal_zone *th_zone = thermal->devdata;
> + int max_trip = th_zone->sensor_conf->trip_data.trip_count;
>  
> - if (trip < GET_TRIP(MONITOR_ZONE) || trip > GET_TRIP(PANIC_ZONE))
> + if (trip < 0 || trip >= max_trip)
>   return -EINVAL;
>  
>   *temp = th_zone->sensor_conf->trip_data.trip_val[trip];
> @@ -112,10 +112,10 @@ static int exynos_get_trip_temp(struct 
> thermal_zone_device *thermal, int trip,
>  static int exynos_get_crit_temp(struct thermal_zone_device *thermal,
>   unsigned long *temp)
>  {
> - int ret;
> - /* Panic zone */
> - ret = exynos_get_trip_temp(thermal, GET_TRIP(PANIC_ZONE), temp);
> - return ret;
> + struct exynos_thermal_zone *th_zone = thermal->devdata;
> + int max_trip = th_zone->sensor_conf->trip_data.trip_count;
> + /* Get the temp of highest trip*/
> + return exynos_get_trip_temp(thermal, max_trip - 1, temp);
>  }
>  
>  /* Bind callback functions for thermal zone */
> @@ -340,19 +340,22 @@ int exynos_register_thermal(struct thermal_sensor_conf 
> *sensor_conf)
>   return -ENOMEM;
>  
>   th_zone->sensor_conf = sensor_conf;
> - cpumask_set_cpu(0, &mask_val);
> - th_zone->cool_dev[0] = cpufreq_cooling_register(&mask_val);
> - if (IS_ERR(th_zone->cool_dev[0])) {
> - pr_err("Failed to register cpufreq cooling device\n");
> - ret = -EINVAL;
> - goto err_unregister;
> + if (sensor_conf->cooling_data.freq_clip_count > 0) {
> + cpumask_set_cpu(0, &mask_val);
> + th_zone->cool_dev[0] = cpufreq_cooling_register(&mask_val);
> + if (IS_ERR(th_zone->cool_dev[0])) {
> + pr_err("Failed to register cpufreq cooling device\n");
> + ret = -EINVAL;
> + goto err_unregister;
> + }
> + th_zone->cool_dev_size++;
>   }
> - th_zone->cool_dev_size++;
>  
> - th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
> - EXYNOS_ZONE_COUNT, 0, th_zone, &exynos_dev_ops, NULL, 0,
> - sensor_conf->trip_data.trigger_falling ?
> - 0 : IDLE_INTERVAL);
> + th_zone->therm_dev = thermal_zone_device_register(
> + sensor_conf->name, sensor_conf->trip_data.trip_count,
> + 0, th_zone, &exynos_dev_ops, NULL, 0,
> + sensor_conf->trip_data.trigger_falling ? 0 :
> + IDLE_INTERVAL);
>  
> 

Re: Pulls and drive strengths in the pinctrl world

[Linus Walleij]

On Fri, May 17, 2013 at 11:09 AM, Tomasz Figa  wrote:

>> Just add another state, pctldev->hog_shutdown to this, and
>> add an operation pinctrl_force_poweroff() in the same spirit as
>> pinctrl_force_sleep() that we already have.
>>
>> Add a new state to include/linux/pinctrl/pinctrl-state.h:
>> #define PINCTRL_STATE_POWEROFF "poweroff"
>>
>> And define you pin table to hog these pins with the mentioned
>> default and poweroff states.
>>
>> Result: pinctrl core keeps track of your offstate too.
>
> Power down mode settings on our pin controller are completely unrelated to
> normal mode settings. You can set them once in appropriate registers and
> pins are switched to them automatically when the SoC enters sleep mode.

Aha so it's actually automatic sleep modes, not power down
(as in, disconnect the power and then push the "on" button to
get it back up).

Please remember to document it per above in the code and the
device tree, so everybody understands what it is.

> So IMHO in our case power mode settings are just additional pin
> configuration options, next to pull-up/-down and driver strength.

I see. Yes that is different.

You might want to have a debugfs file in your driver for inspecting
them though, that sounds like it could be helpful. I'd recommend
augmenting your .pin_config_dbg_show() callback in the
struct pinconf_ops to display this for each pin, in addition to the
current configuration.

Yours,
Linus Walleij
--
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 10/30] thermal: exynos: Support thermal tripping

[jonghwa3 . lee]

Hi, Amit
On 2013년 05월 14일 18:58, Amit Daniel Kachhap wrote:

> TMU urgently sends active-high signal (thermal trip) to PMU, and thermal
> tripping by hardware logic. Thermal tripping means that PMU cuts off the
> whole power of SoC by controlling external voltage regulator.
> 
> Acked-by: Kukjin Kim 
> Signed-off-by: Jonghwan Choi 
> Signed-off-by: Amit Daniel Kachhap 
> ---
>  drivers/thermal/samsung/exynos_tmu.c  |8 +++-
>  drivers/thermal/samsung/exynos_tmu_data.c |2 ++
>  2 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/thermal/samsung/exynos_tmu.c 
> b/drivers/thermal/samsung/exynos_tmu.c
> index 5f8f189..479d61e 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -84,6 +84,7 @@
>  #define EXYNOS_TMU_CLEAR_FALL_INT(0x111 << 12)
>  #define EXYNOS_TMU_TRIP_MODE_SHIFT   13
>  #define EXYNOS_TMU_TRIP_MODE_MASK0x7
> +#define EXYNOS_TMU_THERM_TRIP_EN_SHIFT   12
>  
>  #define EXYNOS_TMU_INTEN_RISE0_SHIFT 0
>  #define EXYNOS_TMU_INTEN_RISE1_SHIFT 4
> @@ -186,7 +187,7 @@ static int exynos_tmu_initialize(struct platform_device 
> *pdev)
>  {
>   struct exynos_tmu_data *data = platform_get_drvdata(pdev);
>   struct exynos_tmu_platform_data *pdata = data->pdata;
> - unsigned int status, trim_info;
> + unsigned int status, trim_info, con;
>   unsigned int rising_threshold = 0, falling_threshold = 0;
>   int ret = 0, threshold_code, i, trigger_levs = 0;
>  
> @@ -251,6 +252,11 @@ static int exynos_tmu_initialize(struct platform_device 
> *pdev)
>   falling_threshold |=
>   threshold_code << 8 * i;
>   }
> + if (pdata->trigger_type[i] != HW_TRIP)
> + continue;


As you know, HW trip can be used when only the most last level of threshold
temperature is set. (exynos4412 : 4th, exynos 5440 : 5th threshold level). So it
wouldn't work properly, even if we enable HW trip according to pre-defined
trigger type not to HW trip threshold temperature. To enable HW trip, we just
need to check whether if HW trip threshold temperature level is defined.

if (trigger_level[HW_TRIP_LEVEL])
enable HW trip

Thanks,
Jonghwa

> + con = readl(data->base + EXYNOS_TMU_REG_CONTROL);
> + con |= (1 << EXYNOS_TMU_THERM_TRIP_EN_SHIFT);
> + writel(con, data->base + EXYNOS_TMU_REG_CONTROL);
>   }
>  
>   writel(rising_threshold,
> diff --git a/drivers/thermal/samsung/exynos_tmu_data.c 
> b/drivers/thermal/samsung/exynos_tmu_data.c
> index ee6a3c9..6b937f5 100644
> --- a/drivers/thermal/samsung/exynos_tmu_data.c
> +++ b/drivers/thermal/samsung/exynos_tmu_data.c
> @@ -64,6 +64,7 @@ struct exynos_tmu_platform_data const 
> exynos5250_default_tmu_data = {
>   .trigger_levels[0] = 85,
>   .trigger_levels[1] = 103,
>   .trigger_levels[2] = 110,
> + .trigger_levels[3] = 120,
>   .trigger_enable[0] = 1,
>   .trigger_enable[1] = 1,
>   .trigger_enable[2] = 1,
> @@ -71,6 +72,7 @@ struct exynos_tmu_platform_data const 
> exynos5250_default_tmu_data = {
>   .trigger_type[0] = 0,
>   .trigger_type[1] = 0,
>   .trigger_type[2] = 1,
> + .trigger_type[3] = 2,
>   .gain = 8,
>   .reference_voltage = 16,
>   .noise_cancel_mode = 4,


--
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 v9] i2c: exynos5: add High Speed I2C controller driver

[Naveen Krishna Chatradhi]

Adds support for High Speed I2C driver found in Exynos5 and
later SoCs from Samsung.

Driver only supports Device Tree method.

Changes since v1:
1. Added FIFO functionality
2. Added High speed mode functionality
3. Remove SMBUS_QUICK
4. Remove the debugfs functionality
5. Use devm_* functions where ever possible
6. Driver is free from GPIO configs (only supports pinctrl method)
7. Use OF data string "clock-frequency" to get the bus operating frequencies
8. Split the clock divisor calculation function
9. Add resets for the failed transacton cases
10. few other bug fixes and cosmetic changes

Signed-off-by: Taekgyun Ko 
Signed-off-by: Naveen Krishna Chatradhi 
Reviewed-by: Simon Glass 
Tested-by: Andrew Bresticker 
Signed-off-by: Yuvaraj Kumar C D 
Signed-off-by: Andrew Bresticker 
---

Changes since v8
1. improved the device tree bindings description page for i2c-exynos5
2. fixed the return value check for devm_ioremap_resource

 .../devicetree/bindings/i2c/i2c-exynos5.txt|   45 +
 drivers/i2c/busses/Kconfig |7 +
 drivers/i2c/busses/Makefile|1 +
 drivers/i2c/busses/i2c-exynos5.c   |  888 
 4 files changed, 941 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
 create mode 100644 drivers/i2c/busses/i2c-exynos5.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt 
b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
new file mode 100644
index 000..29c01c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-exynos5.txt
@@ -0,0 +1,45 @@
+* Samsung's High Speed I2C controller
+
+The Samsung's High Speed I2C controller is used to interface with I2C devices
+at various speeds ranging from 100khz to 3.4Mhz.
+
+Required properties:
+  - compatible: value should be.
+  -> "samsung,exynos5-hsi2c", for i2c compatible with exynos5 hsi2c.
+  - reg: physical base address of the controller and length of memory mapped
+region.
+  - interrupts: interrupt number to the cpu.
+  - #address-cells: always 1 (for i2c addresses)
+  - #size-cells: always 0
+
+  - Pinctrl:
+- pinctrl-0: Pin control group to be used for this controller.
+- pinctrl-names: Should contain only one value - "default".
+
+Optional properties:
+  - samsung,hs-mode: Mode of operation, High speed or Fast speed mode. If not
+specified, default value is 0.
+  - clock-frequency: Desired operating frequency in Hz of the bus.
+If not specified, the default value in Hz is 10.
+
+Example:
+
+hsi2c@12ca {
+   compatible = "samsung,exynos5-hsi2c";
+   reg = <0x12ca 0x100>;
+   interrupts = <56>;
+   clock-frequency = <10>;
+
+   /* Pinctrl variant begins here */
+   pinctrl-0 = <&i2c4_bus>;
+   pinctrl-names = "default";
+   /* Pinctrl variant ends here */
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   s2mps11_pmic@66 {
+   compatible = "samsung,s2mps11-pmic";
+   reg = <0x66>;
+   };
+};
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index adfee98..49a665f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -434,6 +434,13 @@ config I2C_EG20T
  ML7213/ML7223/ML7831 is companion chip for Intel Atom E6xx series.
  ML7213/ML7223/ML7831 is completely compatible for Intel EG20T PCH.
 
+config I2C_EXYNOS5
+   tristate "Exynos5 high-speed I2C driver"
+   depends on ARCH_EXYNOS5 && OF
+   help
+ Say Y here to include support for high-speed I2C controller in the
+ Exynos5 based Samsung SoCs.
+
 config I2C_GPIO
tristate "GPIO-based bitbanging I2C"
depends on GENERIC_GPIO
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 8f4fc23..b19366c 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -42,6 +42,7 @@ i2c-designware-platform-objs := i2c-designware-platdrv.o
 obj-$(CONFIG_I2C_DESIGNWARE_PCI)   += i2c-designware-pci.o
 i2c-designware-pci-objs := i2c-designware-pcidrv.o
 obj-$(CONFIG_I2C_EG20T)+= i2c-eg20t.o
+obj-$(CONFIG_I2C_EXYNOS5)  += i2c-exynos5.o
 obj-$(CONFIG_I2C_GPIO) += i2c-gpio.o
 obj-$(CONFIG_I2C_HIGHLANDER)   += i2c-highlander.o
 obj-$(CONFIG_I2C_IBM_IIC)  += i2c-ibm_iic.o
diff --git a/drivers/i2c/busses/i2c-exynos5.c b/drivers/i2c/busses/i2c-exynos5.c
new file mode 100644
index 000..33c481d
--- /dev/null
+++ b/drivers/i2c/busses/i2c-exynos5.c
@@ -0,0 +1,888 @@
+/**
+ * i2c-exynos5.c - Samsung Exynos5 I2C Controller Driver
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ *
+ * 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 

Re: Pulls and drive strengths in the pinctrl world

[Tomasz Figa]

Hi Linus,

On Friday 17 of May 2013 10:38:53 Linus Walleij wrote:
> On Thu, May 16, 2013 at 2:03 AM, Doug Anderson  
wrote:
> >> I prefer to put all the static pinctrl configuration in the pinctrl
> >> hog, and only the dynamic stuff in the individual device nodes.
> >> 
> >> I know LinusW won't like this suggestion much though:-)
> 
> (It's not that bad actually...)
> 
> > Ah right!  I forgot about hogs in this case.  That's also reasonable
> > as a solution and is similar to what we've got in the tree for
> > powerdown configuration of pins (I'll try to post this patch soon too,
> > WIP at  and
> > .
> 
> I don't like these Gerrit patches really, it's better to move
> this to the pinctrl core using hogs.
> 
> If you look in drivers/pinctr/core.c you can find this:
> 
> pinctrl_register()
> {
> (...)
> if (!IS_ERR(pctldev->p)) {
> pctldev->hog_default =
> pinctrl_lookup_state(pctldev->p,
> PINCTRL_STATE_DEFAULT); if (IS_ERR(pctldev->hog_default)) {
> dev_dbg(dev, "failed to lookup the default
> state\n"); } else {
> if (pinctrl_select_state(pctldev->p,
> pctldev->hog_default))
> dev_err(dev,
> "failed to select default
> state\n"); }
> 
> pctldev->hog_sleep =
> pinctrl_lookup_state(pctldev->p,
>
> PINCTRL_STATE_SLEEP); if (IS_ERR(pctldev->hog_sleep))
> dev_dbg(dev, "failed to lookup the sleep
> state\n"); }
> 
> Just add another state, pctldev->hog_shutdown to this, and
> add an operation pinctrl_force_poweroff() in the same spirit as
> pinctrl_force_sleep() that we already have.
> 
> Add a new state to include/linux/pinctrl/pinctrl-state.h:
> #define PINCTRL_STATE_POWEROFF "poweroff"
> 
> And define you pin table to hog these pins with the mentioned
> default and poweroff states.
> 
> Result: pinctrl core keeps track of your offstate too.

Power down mode settings on our pin controller are completely unrelated to 
normal mode settings. You can set them once in appropriate registers and 
pins are switched to them automatically when the SoC enters sleep mode.

So IMHO in our case power mode settings are just additional pin 
configuration options, next to pull-up/-down and driver strength.

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: Pulls and drive strengths in the pinctrl world

[Linus Walleij]

On Thu, May 16, 2013 at 2:03 AM, Doug Anderson  wrote:

>> I prefer to put all the static pinctrl configuration in the pinctrl hog,
>> and only the dynamic stuff in the individual device nodes.
>>
>> I know LinusW won't like this suggestion much though:-)

(It's not that bad actually...)

> Ah right!  I forgot about hogs in this case.  That's also reasonable
> as a solution and is similar to what we've got in the tree for
> powerdown configuration of pins (I'll try to post this patch soon too,
> WIP at  and
> .

I don't like these Gerrit patches really, it's better to move
this to the pinctrl core using hogs.

If you look in drivers/pinctr/core.c you can find this:

pinctrl_register()
{
(...)
if (!IS_ERR(pctldev->p)) {
pctldev->hog_default =
pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
if (IS_ERR(pctldev->hog_default)) {
dev_dbg(dev, "failed to lookup the default state\n");
} else {
if (pinctrl_select_state(pctldev->p,
pctldev->hog_default))
dev_err(dev,
"failed to select default state\n");
}

pctldev->hog_sleep =
pinctrl_lookup_state(pctldev->p,
PINCTRL_STATE_SLEEP);
if (IS_ERR(pctldev->hog_sleep))
dev_dbg(dev, "failed to lookup the sleep state\n");
}

Just add another state, pctldev->hog_shutdown to this, and
add an operation pinctrl_force_poweroff() in the same spirit as
pinctrl_force_sleep() that we already have.

Add a new state to include/linux/pinctrl/pinctrl-state.h:
#define PINCTRL_STATE_POWEROFF "poweroff"

And define you pin table to hog these pins with the mentioned
default and poweroff states.

Result: pinctrl core keeps track of your offstate too.

Yours,
Linus Walleij
--
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