Re: [PATCH 02/16] pinctrl: exynos: Parse wakeup-eint parameters from DT

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 This patch converts the pinctrl-exynos driver to parse wakeup interrupt
 count and register offsets from device tree. It reduces the amount of
 static platform-specific data and facilitates adding further SoC
 variants to pinctrl-samsung driver.

So these are:

 +   ret = of_property_read_u32(wkup_np, samsung,weint-count, val);
 +   ret = of_property_read_u32(wkup_np, samsung,weint-con, val);
 +   ret = of_property_read_u32(wkup_np, samsung,weint-mask, val);
 +   ret = of_property_read_u32(wkup_np, samsung,weint-pend, val);

Are these all four register offsets?

I don't think it's proper for the device tree to contain register offsets.

Base address, regs property, yes. Individual registers, no. That just
makes the code hard to read and compare to the datasheet.

Or what are you aiming at here?

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 01/16] ARM: dts: exynos4210: Replace legacy GPIO bank nodes with pinctrl bank nodes

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 Seuqential patches from this series introduce SoC-specific data parsing
 from device tree.

 This patch removes legacy GPIO bank nodes from exynos4210.dtsi and
 replaces them with nodes and properties required for these patches.

So to be clear:

 +   pinctrl-bank-types {
 +   bank_off: bank-off {
 +   samsung,reg-names = func, dat, pud,
 +   drv, conpdn, pudpdn;
 +   samsung,reg-params = 0x00 4, 0x04 1, 0x08 2,
 +   0x0C 2, 0x10 2, 0x14 2;
 +   };

This is starting to look like a firmware language, I have mixed
feelings about this. Shall this be read:

Poke 4 into 0x00, poke 1 into 0x04, poke 2 into 0x08 etc?

We really need to discuss this, Grant has already NACK:ed
such approaches once.

If you're still going to do this, it is mandatory
to NOT use magic hex numbers anymore, because Stephen has
merged preprocessor support to the DTC compiler so you
can use #defined macros.

See commit:
cd296721a9645f9f28800a072490fa15458d1fb7

 +   pinctrl@1140 {
 +   gpa0: gpa0 {
 +   gpio-controller;
 +   samsung,pctl-offset = 0x000;
 +   samsung,pin-count = 8;
 +   samsung,bank-type = bank_off;
 +   #gpio-cells = 2;

This part is OK.

 +
 +   interrupt-controller;
 +   samsung,eint-offset = 0x00;

This property is *NOT* OK. IMHO the driver should know these
offsets, not the device tree. The driver only needs the offset to
the register range, what registers there are and their names
should be #defined.

 +   #interrupt-cells = 2;
 +   };

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 04/16] pinctrl: samsung: Parse pin banks from DT

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 Currently SoC-specific properties such as list of pin banks, register
 offsets and bitfield sizes are being taken from static data structures
 residing in pinctrl-exynos.c.

 This patch modifies the pinctrl-samsung driver to parse all SoC-specific
 data from device tree, which will allow to remove the static data
 structures and facilitate adding of further SoC variants to the
 pinctrl-samsung driver.

So why? Two approaches:

- Put as much info as possible into the device tree
- Put as much info as possible into the driver

The first approach is currently only used by pinctrl-single.c.

That driver is designed for the case where all info about
the hardware arrives in some description language that
can be translated into a simple DT description.

If you want to use that approach, you should use that
driver. If that driver does not work for you, then it's not
fulfilling it's purpose as a one-stop shop for simple
pin controllers entirely contained within the device tree,
and should be renamed or redesigned.

If you will end up with a hybrid approach with some
stuff in the device tree and some stuff in the code,
it's better to keep the old driver.

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 03/16] pinctrl: samsung: Detect and handle unsupported configuration types

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 This patch modifies the pinctrl-samsung driver to detect when width of a
 bit field is set to zero (which means that such configuraton type is not
 supported) and return an error instead of trying to modify an inexistent
 register.

 Signed-off-by: Tomasz Figa t.f...@samsung.com
 ---
  drivers/pinctrl/pinctrl-samsung.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/drivers/pinctrl/pinctrl-samsung.c 
 b/drivers/pinctrl/pinctrl-samsung.c
 index dd108a9..c660fa5 100644
 --- a/drivers/pinctrl/pinctrl-samsung.c
 +++ b/drivers/pinctrl/pinctrl-samsung.c
 @@ -391,6 +391,9 @@ static int samsung_pinconf_rw(struct pinctrl_dev 
 *pctldev, unsigned int pin,
 return -EINVAL;
 }

 +   if (!width)
 +   return -EINVAL;
 +

Can this patch be applied in isolation from the others?
Thomas A: can you ACK this so I can apply it in that case?

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 09/16] pinctrl: exynos: Use one IRQ domain per pin bank

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 Instead of registering one IRQ domain for all pin banks of a pin
 controller, this patch implements registration of per-bank domains.

 At a cost of a little memory overhead (~2.5KiB for all GPIO interrupts
 of Exynos4x12) it simplifies driver code and device tree sources,
 because GPIO interrupts can be now specified per banks.

 Example:
 device {
 /* ... */
 interrupt-parent = gpa1;
 interrupts = 3 0;
 /* ... */
 };

 Signed-off-by: Tomasz Figa t.f...@samsung.com

This looks like a very good patch!
Can it be applied in isolation from the other patches?
Thomas A: can you ACK this?

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 10/16] pinctrl: samsung: Do not pass gpio_chip to pin_to_reg_bank

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 The pointer to gpio_chip passed to pin_to_reg_bank utility function is
 used only to retrieve a pointer to samsung_pinctrl_drv_data structure.

 This patch modifies the function and its users to pass a pointer to
 samsung_pinctrl_drv_data directly.

 Signed-off-by: Tomasz Figa t.f...@samsung.com

Looks good, can it be applied without the others?

Maybe you can make a patch series without all the
stuff moving register offsets to the DT so I can begin
with merging that and we can discuss the movement
of register info separately?

Thomas A: is this ACK:able?

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 11/16] pinctrl: samsung: Use one GPIO chip per pin bank

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 This patch modifies the pinctrl-samsung driver to register one GPIO chip
 per pin bank, instead of a single chip for all pin banks of the
 controller.

 It simplifies GPIO accesses a lot (constant time instead of looping
 through the list of banks to find the right one) and should have a good
 effect on performance of any bit-banging driver.

 In addition it allows to reference GPIO pins by a phandle to the bank
 node and a local pin offset inside of the bank (similar to previous
 gpiolib driver), which is more clear and readable than using indices
 relative to the whole pin controller.

 Example:
 device {
 /* ... */
 gpios = gpk0 4 0;
 /* ... */
 };

 Signed-off-by: Tomasz Figa t.f...@samsung.com

This also looks good (and I think it has been discussed before)
so needs to be applied in isolation from the regs-to-DT stuff.

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 00/16] pinctrl: samsung: Usability and extensibiltiy improvements

2012-10-10 Thread Linus Walleij
On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:

 This patch series is a work on improving usability and extensibiltiy of the
 pinctrl-samsung driver. It consists of three main parts:
  - moving SoC-specific data to device tree
  - converting the driver to use one GPIO chip and one IRQ domain per pin bank
  - introducing generic wake-up interrupt capability description

So can you prepare a patch series which does all but the first
bullet to begin with, and a SoC-and register offset patch
on top of that as a separate series, because it is controversial?

I don't like that these two things are mingled together like this
in an all-or-nothing manner.

So I'm OK with a patch series for bulle (2) and (3) but not (1).

And I'd like to have Thomas A:s ACK on the series 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


Re: [PATCH 01/16] ARM: dts: exynos4210: Replace legacy GPIO bank nodes with pinctrl bank nodes

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:26:51 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  Seuqential patches from this series introduce SoC-specific data parsing
  from device tree.
  
  This patch removes legacy GPIO bank nodes from exynos4210.dtsi and
  replaces them with nodes and properties required for these patches.
 
 So to be clear:
  +   pinctrl-bank-types {
  +   bank_off: bank-off {
  +   samsung,reg-names = func, dat, pud,
  +   drv, conpdn,
  pudpdn; +   samsung,reg-params = 0x00 4, 0x04
  1, 0x08 2, +   0x0C
  2, 0x10 2, 0x14 2; +   };
 
 This is starting to look like a firmware language, I have mixed
 feelings about this. Shall this be read:
 
 Poke 4 into 0x00, poke 1 into 0x04, poke 2 into 0x08 etc?

I'm not sure if I understood you correctly, so let me explain how this 
works.

Each specifier defines register offset inside bank registers and how many 
bits are used for one pin in this register to specify configuration value.

E.g. func register is available at offset 0x00 and pin 0 occupies bits 0-3, 
pin 1 bit 4-7, etc.

 
 We really need to discuss this, Grant has already NACK:ed
 such approaches once.
 
 If you're still going to do this, it is mandatory
 to NOT use magic hex numbers anymore, because Stephen has
 merged preprocessor support to the DTC compiler so you
 can use #defined macros.
 
 See commit:
 cd296721a9645f9f28800a072490fa15458d1fb7

That's definitely nice. I have seen the work going on this before, but 
haven't followed it recently. Good to know that now it can be used.
 
  +   pinctrl@1140 {
  +   gpa0: gpa0 {
  +   gpio-controller;
  +   samsung,pctl-offset = 0x000;
  +   samsung,pin-count = 8;
  +   samsung,bank-type = bank_off;
  +   #gpio-cells = 2;
 
 This part is OK.
 
  +
  +   interrupt-controller;
  +   samsung,eint-offset = 0x00;
 
 This property is *NOT* OK. IMHO the driver should know these
 offsets, not the device tree. The driver only needs the offset to
 the register range, what registers there are and their names
 should be #defined.

This is an offset inside of EINT register group. EINT registers are 
organized in groups as following:

EINT_CON_0
EINT_CON_1
...
EINT_CON_N

...

EINT_MASK_0
EINT_MASK_1
...
EINT_MASK_N

...

EINT_PEND_0
EINT_PEND_1
...
EINT_PEND_N

With arbitrary order of particular groups, arbitrary space between groups 
and arbitrary mapping of particular registers to pin banks, although the 
mapping is the same for all groups of registers, that's why there is only 
one eint-offset property. Also holes (reserved/unused registers) inside 
groups might exist.

So if we want to access EINT_MASK register of bank A0 (of pinctrl 0), we 
must construct the address as following:

eint_mask_a0 = pinctrl_0_base + pinctrl_0_geint_mask + bank_a0_eint_offset

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 02/16] pinctrl: exynos: Parse wakeup-eint parameters from DT

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:18:51 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  This patch converts the pinctrl-exynos driver to parse wakeup interrupt
  count and register offsets from device tree. It reduces the amount of
  static platform-specific data and facilitates adding further SoC
  variants to pinctrl-samsung driver.
 
 So these are:
  +   ret = of_property_read_u32(wkup_np, samsung,weint-count,
  val); +   ret = of_property_read_u32(wkup_np,
  samsung,weint-con, val); +   ret =
  of_property_read_u32(wkup_np, samsung,weint-mask, val); +   ret
  = of_property_read_u32(wkup_np, samsung,weint-pend, val);
 Are these all four register offsets?
 
 I don't think it's proper for the device tree to contain register
 offsets.
 
 Base address, regs property, yes. Individual registers, no. That just
 makes the code hard to read and compare to the datasheet.
 
 Or what are you aiming at here?

See my reply to your comments for patch 1. I think it should explain how 
these values are used.

One thing worth mentioning is that registers for GPIO interrupts and wakeup 
interrupts can be located at different areas of pin controller address 
space, so these offsets have to be specified for both (using geint-* and 
weint-* properties).

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 03/16] pinctrl: samsung: Detect and handle unsupported configuration types

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:37:42 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  This patch modifies the pinctrl-samsung driver to detect when width of
  a
  bit field is set to zero (which means that such configuraton type is
  not
  supported) and return an error instead of trying to modify an
  inexistent
  register.
  
  Signed-off-by: Tomasz Figa t.f...@samsung.com
  ---
  
   drivers/pinctrl/pinctrl-samsung.c | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/drivers/pinctrl/pinctrl-samsung.c
  b/drivers/pinctrl/pinctrl-samsung.c index dd108a9..c660fa5 100644
  --- a/drivers/pinctrl/pinctrl-samsung.c
  +++ b/drivers/pinctrl/pinctrl-samsung.c
  @@ -391,6 +391,9 @@ static int samsung_pinconf_rw(struct pinctrl_dev
  *pctldev, unsigned int pin, 
  return -EINVAL;
  
  }
  
  +   if (!width)
  +   return -EINVAL;
  +
 
 Can this patch be applied in isolation from the others?

Yes, I don't see any problem here.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 04/16] pinctrl: samsung: Parse pin banks from DT

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:34:05 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  Currently SoC-specific properties such as list of pin banks, register
  offsets and bitfield sizes are being taken from static data structures
  residing in pinctrl-exynos.c.
  
  This patch modifies the pinctrl-samsung driver to parse all
  SoC-specific
  data from device tree, which will allow to remove the static data
  structures and facilitate adding of further SoC variants to the
  pinctrl-samsung driver.
 
 So why? Two approaches:
 
 - Put as much info as possible into the device tree
 - Put as much info as possible into the driver
 
 The first approach is currently only used by pinctrl-single.c.
 
 That driver is designed for the case where all info about
 the hardware arrives in some description language that
 can be translated into a simple DT description.
 
 If you want to use that approach, you should use that
 driver. If that driver does not work for you, then it's not
 fulfilling it's purpose as a one-stop shop for simple
 pin controllers entirely contained within the device tree,
 and should be renamed or redesigned.
 
 If you will end up with a hybrid approach with some
 stuff in the device tree and some stuff in the code,
 it's better to keep the old driver.

This will allow us to cover all the existing Samsung SoCs, starting from 
S3C24xx, through S3C64xx, S5P*, all supported Exynos SoCs and ending on any 
future SoCs using this kind of pin controller, without bloating the driver 
with hardly readable macros, lots of (often duplicated) static data and 
similar.

If there are some serious problems with this approach, just let me know and 
I will reconsider it, but if not, I'd like to keep it, because of the 
benefits it gives.

(Even if I moved all those SoC-specific data to static structures located 
in the driver, to keep the most readable way of GPIO specification in DT, I 
would have to create nodes for all banks in DT anyway and the driver would 
have to match particular nodes with their static data. I don't like this 
kind of approach.)

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 09/16] pinctrl: exynos: Use one IRQ domain per pin bank

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:40:16 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  Instead of registering one IRQ domain for all pin banks of a pin
  controller, this patch implements registration of per-bank domains.
  
  At a cost of a little memory overhead (~2.5KiB for all GPIO interrupts
  of Exynos4x12) it simplifies driver code and device tree sources,
  because GPIO interrupts can be now specified per banks.
  
  Example:
  device {
  
  /* ... */
  interrupt-parent = gpa1;
  interrupts = 3 0;
  /* ... */
  
  };
  
  Signed-off-by: Tomasz Figa t.f...@samsung.com
 
 This looks like a very good patch!
 Can it be applied in isolation from the other patches?

This is heavily dependent on previous patches, because each pin bank must 
have its own node that can be bound to the IRQ domain and used as an 
interrupt-controller in interrupt-parent property.

I can imagine kind of hybrid solution, where bank nodes contain almost no 
data, other than gpio-controller, interrupt-controller and #*-cells 
properties, but this would introduce the need of matching bank nodes with 
banks statically defined in the driver.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 11/16] pinctrl: samsung: Use one GPIO chip per pin bank

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:43:25 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  This patch modifies the pinctrl-samsung driver to register one GPIO
  chip
  per pin bank, instead of a single chip for all pin banks of the
  controller.
  
  It simplifies GPIO accesses a lot (constant time instead of looping
  through the list of banks to find the right one) and should have a good
  effect on performance of any bit-banging driver.
  
  In addition it allows to reference GPIO pins by a phandle to the bank
  node and a local pin offset inside of the bank (similar to previous
  gpiolib driver), which is more clear and readable than using indices
  relative to the whole pin controller.
  
  Example:
  device {
  
  /* ... */
  gpios = gpk0 4 0;
  /* ... */
  
  };
  
  Signed-off-by: Tomasz Figa t.f...@samsung.com
 
 This also looks good (and I think it has been discussed before)
 so needs to be applied in isolation from the regs-to-DT stuff.

Please see my reply for patch 9.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 10/16] pinctrl: samsung: Do not pass gpio_chip to pin_to_reg_bank

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:42:10 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  The pointer to gpio_chip passed to pin_to_reg_bank utility function is
  used only to retrieve a pointer to samsung_pinctrl_drv_data structure.
  
  This patch modifies the function and its users to pass a pointer to
  samsung_pinctrl_drv_data directly.
  
  Signed-off-by: Tomasz Figa t.f...@samsung.com
 
 Looks good, can it be applied without the others?

Yes, I think this one should apply fine.

 Maybe you can make a patch series without all the
 stuff moving register offsets to the DT so I can begin
 with merging that and we can discuss the movement
 of register info separately?

I already considered this approach, but it introduces some problems, as I 
mentioned in my other replies.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 00/16] pinctrl: samsung: Usability and extensibiltiy improvements

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:46:28 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  This patch series is a work on improving usability and extensibiltiy of
  the 
  pinctrl-samsung driver. It consists of three main parts:
   - moving SoC-specific data to device tree
   - converting the driver to use one GPIO chip and one IRQ domain per
   pin bank - introducing generic wake-up interrupt capability
   description
 
 So can you prepare a patch series which does all but the first
 bullet to begin with, and a SoC-and register offset patch
 on top of that as a separate series, because it is controversial?
 
 I don't like that these two things are mingled together like this
 in an all-or-nothing manner.
 
 So I'm OK with a patch series for bulle (2) and (3) but not (1).

As I already stated in my other replies, patches for (2) and (3) depend on 
(1), because current architecture of the driver makes it hard to implement 
in an elegant way, so some code would have to be reworked anyway.

I should still have some preliminary work on this in one of my branches in 
my working tree, but it was heavily incomplete, as it turned out that using 
DT would simplify some things.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY

2012-10-10 Thread Vasanth Ananthan
Hi,

On Tue, Oct 9, 2012 at 11:58 PM, Olof Johansson o...@lixom.net wrote:

 Hi,


 On Tue, Oct 09, 2012 at 05:18:48PM +0530, Vasanth Ananthan wrote:
  This patch adds Device Nodes for SATA and SATA PHY device.
 
  Signed-off-by: Vasanth Ananthan vasant...@samsung.com
  ---
   arch/arm/boot/dts/exynos5250-smdk5250.dts |   11 +++
   arch/arm/boot/dts/exynos5250.dtsi |   20 
   arch/arm/mach-exynos/include/mach/map.h   |7 +++
   arch/arm/mach-exynos/mach-exynos5-dt.c|6 ++
   4 files changed, 44 insertions(+), 0 deletions(-)
 
  diff --git a/arch/arm/boot/dts/exynos5250-smdk5250.dts 
  b/arch/arm/boot/dts/exynos5250-smdk5250.dts
  index 8a5e348..bb262ce 100644
  --- a/arch/arm/boot/dts/exynos5250-smdk5250.dts
  +++ b/arch/arm/boot/dts/exynos5250-smdk5250.dts
  @@ -48,6 +48,17 @@
};
};
 
  + i2c@121D {
  + samsung,i2c-sda-delay = 100;
  +samsung,i2c-max-bus-freq = 4;
  + samsung,i2c-slave-addr = 0x38;

 Whitespace is off above.

  +
  + sataphy@70 {

 sata-phy would be a more conventional name.

  + compatible = samsung,i2c-phy;

 i2c-phy? Seems like an odd choice of name. What is this device?


The SATA physical layer controller is both a platform device and a i2c
slave device.
This compatible string is for the i2c client driver. Hence I have used
i2c-phy here.



  + reg = 0x38;

 70 is unit address but here it's 0x38? One of them is wrong. No need for a 
 unit
 address if it's a unique name, by the way.


  + };
  + };
  +
i2c@12C8 {
status = disabled;
};
  diff --git a/arch/arm/boot/dts/exynos5250.dtsi 
  b/arch/arm/boot/dts/exynos5250.dtsi
  index 004aaa8..5a47a8f 100644
  --- a/arch/arm/boot/dts/exynos5250.dtsi
  +++ b/arch/arm/boot/dts/exynos5250.dtsi
  @@ -88,6 +88,18 @@
interrupts = 0 54 0;
};
 
  + sata@122F {
  +compatible = samsung,exynos-sata-ahci;
  +reg = 0x122F 0x1ff;
  + interrupts = 0 115 0;

 More whitespace damage. And need binding.

  +};
  +
  +sata-phy@1217 {
  +compatible = samsung,exynos-sata-phy;
  +reg = 0x1217 0x1ff;
  +};

 Should have binding too. How does this relate to the i2c device above.


As mentioned earlier SATA physical layer controller is both a platform
device and also an i2c slave device.
This Node is for the SATA physical layer platform device, the previous
node is for i2c slave device.
Certain initialization settings done directly and other settings has
to be done through i2c.


  +
i2c@12C6 {
compatible = samsung,s3c2440-i2c;
reg = 0x12C6 0x100;
  @@ -152,6 +164,13 @@
#size-cells = 0;
};
 
  + i2c@121D {
  +compatible = samsung,s3c2440-sataphy-i2c;

 Is this a unique i2c controller, or is it just another one like the others on
 the chip? If it's the latter, it should use the regular compatible string.


Yes, its a unique i2c controller which lacks an interrupt line while
others are interrupt driven.
Hence I have used a distinct compatible string for the driver to
distinguish the controller.


  +reg = 0x121D 0x100;
  +#address-cells = 1;
  +#size-cells = 0;
  +};
  +
spi_0: spi@12d2 {
compatible = samsung,exynos4210-spi;
reg = 0x12d2 0x100;
  @@ -460,4 +479,5 @@
#gpio-cells = 4;
};
};
  +

 Stray whitespace change.

   };
  diff --git a/arch/arm/mach-exynos/include/mach/map.h 
  b/arch/arm/mach-exynos/include/mach/map.h
  index c72b675..6827190 100644
  --- a/arch/arm/mach-exynos/include/mach/map.h
  +++ b/arch/arm/mach-exynos/include/mach/map.h
  @@ -177,9 +177,16 @@
   #define EXYNOS4_PA_HSOTG 0x1248
   #define EXYNOS4_PA_USB_HSPHY 0x125B
 
  +#ifdef CONFIG_ARCH_EXYNOS4

 No need to ifdef since namespace isn't overlapped.

   #define EXYNOS4_PA_SATA  0x1256
   #define EXYNOS4_PA_SATAPHY   0x125D
   #define EXYNOS4_PA_SATAPHY_CTRL  0x126B
  +#endif
  +#ifdef CONFIG_ARCH_EXYNOS5

 Same here.

  +#define EXYNOS5_PA_SATA_PHY_CTRL 0x1217
  +#define EXYNOS5_PA_SATA_PHY_I2C  0x121D
  +#define EXYNOS5_PA_SATA_BASE 0x122F
  +#endif


 -Olof


I will incorporate the other comments and resubmit the patch. Thanks.

--
Vasanth Ananthan.
--
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/4 v2] Adding usb2.0 host-phy support for exynos5250

2012-10-10 Thread Vivek Gautam
This patchset is based on the work by Praveen Paneri for
samsung-usbphy driver:
http://comments.gmane.org/gmane.linux.kernel.samsung-soc/12653

Changes from v1:
 - squashed the patch ARM: S3C64XX: Add phy_type to pmu_isolation into
   usb: phy: samsung: Add host phy support to samsung-phy driver.
 - moved similar change of adding phy_type to pmu_isolation for exynos from
   ARM: Exynos5250: Enabling samsung-usbphy driver to
   usb: phy: samsung: Add host phy support to samsung-phy driver.
 - moved s5p_usb_phy_pmu_isolation() declaration from
   ARM: Exynos5250: Enabling samsung-usbphy driver to
   usb: phy: samsung: Add host phy support to samsung-phy driver.
 - moved phy_cfg_sel function pointer declaration from
   usb: phy: samsung: Add host phy support to samsung-phy driver to
   ARM: Exynos5250: Enabling samsung-usbphy driver.
 - Replaced the patch subject usb: s5p-ehci: Adding phy driver support and
   usb: exynos-ohci: Adding phy driver support with
   USB: ehci-s5p: Add phy driver support and
   USB: ohci-exynos: Add phy driver support respectively.
 - Corrected the header include order in ehci-s5p and ohci-exynos.
 - Corrected wrong error path for s5p_ehci_phy_disable() and
   exynos_ohci_phy_disable() in ehci-s5p and ohci-exynos respectively.

Tested on smdk5250 target with usb-next branch along with arch patches
for exynos5250:
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/13042
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/13048

Vivek Gautam (4):
  usb: phy: samsung: Add host phy support to samsung-phy driver
  ARM: Exynos5250: Enabling samsung-usbphy driver
  USB: ehci-s5p: Add phy driver support
  USB: ohci-exynos: Add phy driver support

 .../devicetree/bindings/usb/samsung-usbphy.txt |   12 +-
 arch/arm/boot/dts/exynos5250.dtsi  |5 +
 arch/arm/mach-exynos/Kconfig   |1 +
 arch/arm/mach-exynos/include/mach/map.h|1 +
 arch/arm/mach-exynos/mach-exynos5-dt.c |   10 +
 arch/arm/mach-exynos/setup-usb-phy.c   |   45 ++-
 arch/arm/mach-s3c64xx/setup-usb-phy.c  |2 +-
 arch/arm/plat-samsung/include/plat/usb-phy.h   |3 +-
 drivers/usb/host/ehci-s5p.c|   65 +++-
 drivers/usb/host/ohci-exynos.c |   65 +++-
 drivers/usb/phy/Kconfig|1 -
 drivers/usb/phy/samsung-usbphy.c   |  368 ++--
 include/linux/platform_data/samsung-usbphy.h   |9 +-
 13 files changed, 507 insertions(+), 80 deletions(-)

-- 
1.7.6.5

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


[PATCH 1/4 v2] usb: phy: samsung: Add host phy support to samsung-phy driver

2012-10-10 Thread Vivek Gautam
This patch adds host phy support to samsung-usbphy.c and
further adds support for samsung's exynos5250 usb-phy.

Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
 arch/arm/mach-exynos/setup-usb-phy.c |2 +-
 arch/arm/mach-s3c64xx/setup-usb-phy.c|2 +-
 arch/arm/plat-samsung/include/plat/usb-phy.h |2 +-
 drivers/usb/phy/Kconfig  |1 -
 drivers/usb/phy/samsung-usbphy.c |  368 --
 include/linux/platform_data/samsung-usbphy.h |8 +-
 6 files changed, 352 insertions(+), 31 deletions(-)

diff --git a/arch/arm/mach-exynos/setup-usb-phy.c 
b/arch/arm/mach-exynos/setup-usb-phy.c
index 1c62d20..be6cd4f 100644
--- a/arch/arm/mach-exynos/setup-usb-phy.c
+++ b/arch/arm/mach-exynos/setup-usb-phy.c
@@ -222,7 +222,7 @@ int s5p_usb_phy_exit(struct platform_device *pdev, int type)
return -EINVAL;
 }
 
-void s5p_usb_phy_pmu_isolation(int on)
+void s5p_usb_phy_pmu_isolation(int on, int type)
 {
if (on) {
writel(readl(S5P_USBDEVICE_PHY_CONTROL)
diff --git a/arch/arm/mach-s3c64xx/setup-usb-phy.c 
b/arch/arm/mach-s3c64xx/setup-usb-phy.c
index 3aee778..b7d1d95 100644
--- a/arch/arm/mach-s3c64xx/setup-usb-phy.c
+++ b/arch/arm/mach-s3c64xx/setup-usb-phy.c
@@ -13,7 +13,7 @@
 #include mach/map.h
 #include mach/regs-sys.h
 
-void s5p_usb_phy_pmu_isolation(int on)
+void s5p_usb_phy_pmu_isolation(int on, int type)
 {
if (on) {
writel(readl(S3C64XX_OTHERS)  ~S3C64XX_OTHERS_USBMASK,
diff --git a/arch/arm/plat-samsung/include/plat/usb-phy.h 
b/arch/arm/plat-samsung/include/plat/usb-phy.h
index 165ffe7..7a4a959 100644
--- a/arch/arm/plat-samsung/include/plat/usb-phy.h
+++ b/arch/arm/plat-samsung/include/plat/usb-phy.h
@@ -18,6 +18,6 @@ enum s5p_usb_phy_type {
 
 extern int s5p_usb_phy_init(struct platform_device *pdev, int type);
 extern int s5p_usb_phy_exit(struct platform_device *pdev, int type);
-extern void s5p_usb_phy_pmu_isolation(int on);
+extern void s5p_usb_phy_pmu_isolation(int on, int type);
 
 #endif /* __PLAT_SAMSUNG_USB_PHY_H */
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 313685f..1ce5b32 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -35,7 +35,6 @@ config MV_U3D_PHY
 
 config SAMSUNG_USBPHY
bool Samsung USB PHY controller Driver
-   depends on USB_S3C_HSOTG
select USB_OTG_UTILS
help
  Enable this to support Samsung USB phy controller for samsung
diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c
index ee2dee0..bd6a5e8 100644
--- a/drivers/usb/phy/samsung-usbphy.c
+++ b/drivers/usb/phy/samsung-usbphy.c
@@ -61,9 +61,120 @@
 #define MHZ (1000*1000)
 #endif
 
+/* EXYNOS5 */
+#define EXYNOS5_PHYHOST(0x00)
+
+#define EXYNOS5_PHYHOST_PHYSWRSTALL(0x1  31)
+
+#define EXYNOS5_PHYHOST_REFCLKSEL_MASK (0x3)
+#define EXYNOS5_PHYHOST_REFCLKSEL(_x)  ((_x)  19)
+#define EXYNOS5_PHYHOST_REFCLKSEL_XTAL \
+   EXYNOS5_PHYHOST_REFCLKSEL(0x0)
+#define EXYNOS5_PHYHOST_REFCLKSEL_EXTL \
+   EXYNOS5_PHYHOST_REFCLKSEL(0x1)
+#define EXYNOS5_PHYHOST_REFCLKSEL_CLKCORE  \
+   EXYNOS5_PHYHOST_REFCLKSEL(0x2)
+
+#define EXYNOS5_PHYHOST_FSEL_MASK  (0x7  16)
+#define EXYNOS5_PHYHOST_FSEL(_x)   ((_x)  16)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_50M(0x7)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_24M(0x5)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_20M(0x4)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_19200K (0x3)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_12M(0x2)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_10M(0x1)
+#define EXYNOS5_PHYHOST_FSEL_CLKSEL_9600K  (0x0)
+
+#define EXYNOS5_PHYHOST_TESTBURNIN (0x1  11)
+#define EXYNOS5_PHYHOST_RETENABLE  (0x1  10)
+#define EXYNOS5_PHYHOST_COMMONON_N (0x1  9)
+#define EXYNOS5_PHYHOST_SIDDQ  (0x1  6)
+#define EXYNOS5_PHYHOST_FORCESLEEP (0x1  5)
+#define EXYNOS5_PHYHOST_FORCESUSPEND   (0x1  4)
+#define EXYNOS5_PHYHOST_WORDINTERFACE  (0x1  3)
+#define EXYNOS5_PHYHOST_UTMISWRST  (0x1  2)
+#define EXYNOS5_PHYHOST_LINKSWRST  (0x1  1)
+#define EXYNOS5_PHYHOST_PHYSWRST   (0x1  0)
+
+#define EXYNOS5_PHYHOST_TUNE0  (0x04)
+
+#define EXYNOS5_PHYHSIC1   (0x10)
+
+#define EXYNOS5_PHYHSIC_TUNE1  (0x14)
+
+#define EXYNOS5_PHYHSIC2   (0x20)
+
+#define EXYNOS5_PHYHSIC_TUNE2  (0x24)
+
+#define EXYNOS5_PHYHSIC_REFCLKSEL_MASK (0x3)
+#define EXYNOS5_PHYHSIC_REFCLKSEL  (0x2  23)
+
+#define EXYNOS5_PHYHSIC_REFCLKDIV_MASK (0x7f)
+#define EXYNOS5_PHYHSIC_REFCLKDIV(_x)  ((_x)  16)
+#define EXYNOS5_PHYHSIC_REFCLKDIV_12   \
+   

[PATCH 3/4 v2] USB: ehci-s5p: Add phy driver support

2012-10-10 Thread Vivek Gautam
Adding the transceiver to ehci driver. Keeping the platform data
for continuing the smooth operation for boards which still uses it

Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
 drivers/usb/host/ehci-s5p.c |   65 +-
 1 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/ehci-s5p.c b/drivers/usb/host/ehci-s5p.c
index 85b74be..6dac38f 100644
--- a/drivers/usb/host/ehci-s5p.c
+++ b/drivers/usb/host/ehci-s5p.c
@@ -17,6 +17,7 @@
 #include linux/platform_device.h
 #include linux/of_gpio.h
 #include linux/platform_data/usb-ehci-s5p.h
+#include linux/usb/phy.h
 #include plat/usb-phy.h
 
 #define EHCI_INSNREG00(base)   (base + 0x90)
@@ -32,6 +33,8 @@ struct s5p_ehci_hcd {
struct device *dev;
struct usb_hcd *hcd;
struct clk *clk;
+   struct usb_phy *phy;
+   struct s5p_ehci_platdata *pdata;
 };
 
 static const struct hc_driver s5p_ehci_hc_driver = {
@@ -65,6 +68,26 @@ static const struct hc_driver s5p_ehci_hc_driver = {
.clear_tt_buffer_complete   = ehci_clear_tt_buffer_complete,
 };
 
+static void s5p_ehci_phy_enable(struct s5p_ehci_hcd *s5p_ehci)
+{
+   struct platform_device *pdev = to_platform_device(s5p_ehci-dev);
+
+   if (s5p_ehci-phy)
+   usb_phy_init(s5p_ehci-phy);
+   else if (s5p_ehci-pdata-phy_init)
+   s5p_ehci-pdata-phy_init(pdev, S5P_USB_PHY_HOST);
+}
+
+static void s5p_ehci_phy_disable(struct s5p_ehci_hcd *s5p_ehci)
+{
+   struct platform_device *pdev = to_platform_device(s5p_ehci-dev);
+
+   if (s5p_ehci-phy)
+   usb_phy_shutdown(s5p_ehci-phy);
+   else if (s5p_ehci-pdata-phy_exit)
+   s5p_ehci-pdata-phy_exit(pdev, S5P_USB_PHY_HOST);
+}
+
 static void s5p_setup_vbus_gpio(struct platform_device *pdev)
 {
int err;
@@ -92,15 +115,10 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
struct usb_hcd *hcd;
struct ehci_hcd *ehci;
struct resource *res;
+   struct usb_phy *phy;
int irq;
int err;
 
-   pdata = pdev-dev.platform_data;
-   if (!pdata) {
-   dev_err(pdev-dev, No platform data defined\n);
-   return -EINVAL;
-   }
-
/*
 * Right now device-tree probed devices don't get dma_mask set.
 * Since shared usb code relies on it, set it here for now.
@@ -118,6 +136,20 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
if (!s5p_ehci)
return -ENOMEM;
 
+   pdata = pdev-dev.platform_data;
+   if (!pdata) {
+   /* Fallback to Phy transceiver */
+   phy = devm_usb_get_phy(pdev-dev, USB_PHY_TYPE_USB2);
+   if (IS_ERR_OR_NULL(phy)) {
+   dev_err(pdev-dev, no platform data or transceiver 
defined\n);
+   return -EPROBE_DEFER;
+   } else {
+   s5p_ehci-phy = phy;
+   }
+   } else {
+   s5p_ehci-pdata = pdata;
+   }
+
s5p_ehci-dev = pdev-dev;
 
hcd = usb_create_hcd(s5p_ehci_hc_driver, pdev-dev,
@@ -163,8 +195,7 @@ static int __devinit s5p_ehci_probe(struct platform_device 
*pdev)
goto fail_io;
}
 
-   if (pdata-phy_init)
-   pdata-phy_init(pdev, S5P_USB_PHY_HOST);
+   s5p_ehci_phy_enable(s5p_ehci);
 
ehci = hcd_to_ehci(hcd);
ehci-caps = hcd-regs;
@@ -175,13 +206,15 @@ static int __devinit s5p_ehci_probe(struct 
platform_device *pdev)
err = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (err) {
dev_err(pdev-dev, Failed to add USB HCD\n);
-   goto fail_io;
+   goto fail_add_hcd;
}
 
platform_set_drvdata(pdev, s5p_ehci);
 
return 0;
 
+fail_add_hcd:
+   s5p_ehci_phy_disable(s5p_ehci);
 fail_io:
clk_disable(s5p_ehci-clk);
 fail_clk:
@@ -191,14 +224,12 @@ fail_clk:
 
 static int __devexit s5p_ehci_remove(struct platform_device *pdev)
 {
-   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
struct s5p_ehci_hcd *s5p_ehci = platform_get_drvdata(pdev);
struct usb_hcd *hcd = s5p_ehci-hcd;
 
usb_remove_hcd(hcd);
 
-   if (pdata  pdata-phy_exit)
-   pdata-phy_exit(pdev, S5P_USB_PHY_HOST);
+   s5p_ehci_phy_disable(s5p_ehci);
 
clk_disable(s5p_ehci-clk);
 
@@ -222,14 +253,11 @@ static int s5p_ehci_suspend(struct device *dev)
struct s5p_ehci_hcd *s5p_ehci = dev_get_drvdata(dev);
struct usb_hcd *hcd = s5p_ehci-hcd;
bool do_wakeup = device_may_wakeup(dev);
-   struct platform_device *pdev = to_platform_device(dev);
-   struct s5p_ehci_platdata *pdata = pdev-dev.platform_data;
int rc;
 
rc = ehci_suspend(hcd, do_wakeup);
 
-   if (pdata  pdata-phy_exit)
-   pdata-phy_exit(pdev, S5P_USB_PHY_HOST);
+   

Re: [Linaro-mm-sig] [PATCHv9 00/25] Integration of videobuf2 with DMABUF

2012-10-10 Thread Kyungmin Park
On 10/10/12, Mauro Carvalho Chehab mche...@redhat.com wrote:
 Hi,

 Em Tue, 02 Oct 2012 16:27:11 +0200
 Tomasz Stanislawski t.stanisl...@samsung.com escreveu:

 Hello everyone,
 This patchset adds support for DMABUF [2] importing and exporting to V4L2
 stack.

 v9:
 - rebase on 3.6
 - change type for fs to __s32
 - add support for vb2_ioctl_expbuf
 - remove patch 'v4l: vb2: add support for DMA_ATTR_NO_KERNEL_MAPPING',
   it will be posted as a separate patch
 - fix typos and style in Documentation (from Hans Verkuil)
 - only vb2-core and vb2-dma-contig selects DMA_SHARED_BUFFER in Kconfig
 - use data_offset and length while queueing DMABUF
 - return the most recently used fd at VIDIOC_DQBUF
 - use (buffer-type, index, plane) tuple instead of mem_offset
   to identify a for exporting (from Hans Verkuil)
 - support for DMABUF mmap in vb2-dma-contig (from Laurent Pinchart)
 - add testing alignment of vaddr and size while verifying userptr
   against DMA capabilities (from Laurent Pinchart)
 - substitute VM_DONTDUMP with VM_RESERVED
 - simplify error handling in vb2_dc_get_dmabuf (from Laurent Pinchart)

 For now, NACK. See below.

Sad news!
It's failed to merge by very poor samsung board support at mainline.

CC arm  samsung mailing list.

Thank you,
Kyungmin Park

 I spent 4 days trying to setup an environment that would allow testing
 DMABUF with video4linux without success (long story, see below). Also,
 Laurent tried the same without any luck, and it seems that it even
 corrupted his test machine.

 Basically Samsung generously donated me two boards that it could be
 used on this test (Origen and SMDK310). None of them actually worked
 with the upstream kernel: patches are needed to avoid OOPSes on
 Origen; both Origen/SMDK310 defconfigs are completely broken, and drivers
 don't even boot if someone tries to use the Kernel's defconfigs.

 Even after spending _days_ trying to figure out the needed .config options
 (as the config files are not easily available), both boards have... issues:

   - lack of any display output driver at SMDK310;

   - lack of any network driver at Origen: it seems that none of
 the available network options on Origen was upstreamed: no Bluetooth, no
 OTG,
 no Wifi.

 The only test I was able to do (yesterday, late night), the DMABUF caused
 an OOPS at the Origen board. So, for sure it is not ready for upstream.

 It is now, too late for 3.7. I might consider it to 3.8, if something
 can be done to fix the existing issues, and setup a proper setup, using
 the upstream Kernel.

 Regards,
 Mauro

 ___
 Linaro-mm-sig mailing list
 linaro-mm-...@lists.linaro.org
 http://lists.linaro.org/mailman/listinfo/linaro-mm-sig

--
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] video: s3c-fb: use clk_prepare_enable and clk_disable_unprepare

2012-10-10 Thread Florian Tobias Schandinat
On 10/02/2012 11:57 PM, Thomas Abraham wrote:
 Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
 calls as required by common clock framework.
 
 Signed-off-by: Thomas Abraham thomas.abra...@linaro.org

Applied.


Thanks,

Florian Tobias Schandinat


 ---
  drivers/video/s3c-fb.c |   28 ++--
  1 files changed, 14 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
 index 52b744f..2ed7b63 100644
 --- a/drivers/video/s3c-fb.c
 +++ b/drivers/video/s3c-fb.c
 @@ -1404,7 +1404,7 @@ static int __devinit s3c_fb_probe(struct 
 platform_device *pdev)
   return PTR_ERR(sfb-bus_clk);
   }
  
 - clk_enable(sfb-bus_clk);
 + clk_prepare_enable(sfb-bus_clk);
  
   if (!sfb-variant.has_clksel) {
   sfb-lcd_clk = devm_clk_get(dev, sclk_fimd);
 @@ -1414,7 +1414,7 @@ static int __devinit s3c_fb_probe(struct 
 platform_device *pdev)
   goto err_bus_clk;
   }
  
 - clk_enable(sfb-lcd_clk);
 + clk_prepare_enable(sfb-lcd_clk);
   }
  
   pm_runtime_enable(sfb-dev);
 @@ -1504,10 +1504,10 @@ err_lcd_clk:
   pm_runtime_disable(sfb-dev);
  
   if (!sfb-variant.has_clksel)
 - clk_disable(sfb-lcd_clk);
 + clk_disable_unprepare(sfb-lcd_clk);
  
  err_bus_clk:
 - clk_disable(sfb-bus_clk);
 + clk_disable_unprepare(sfb-bus_clk);
  
   return ret;
  }
 @@ -1531,9 +1531,9 @@ static int __devexit s3c_fb_remove(struct 
 platform_device *pdev)
   s3c_fb_release_win(sfb, sfb-windows[win]);
  
   if (!sfb-variant.has_clksel)
 - clk_disable(sfb-lcd_clk);
 + clk_disable_unprepare(sfb-lcd_clk);
  
 - clk_disable(sfb-bus_clk);
 + clk_disable_unprepare(sfb-bus_clk);
  
   pm_runtime_put_sync(sfb-dev);
   pm_runtime_disable(sfb-dev);
 @@ -1561,9 +1561,9 @@ static int s3c_fb_suspend(struct device *dev)
   }
  
   if (!sfb-variant.has_clksel)
 - clk_disable(sfb-lcd_clk);
 + clk_disable_unprepare(sfb-lcd_clk);
  
 - clk_disable(sfb-bus_clk);
 + clk_disable_unprepare(sfb-bus_clk);
  
   pm_runtime_put_sync(sfb-dev);
  
 @@ -1581,10 +1581,10 @@ static int s3c_fb_resume(struct device *dev)
  
   pm_runtime_get_sync(sfb-dev);
  
 - clk_enable(sfb-bus_clk);
 + clk_prepare_enable(sfb-bus_clk);
  
   if (!sfb-variant.has_clksel)
 - clk_enable(sfb-lcd_clk);
 + clk_prepare_enable(sfb-lcd_clk);
  
   /* setup gpio and output polarity controls */
   pd-setup_gpio();
 @@ -1640,9 +1640,9 @@ static int s3c_fb_runtime_suspend(struct device *dev)
   struct s3c_fb *sfb = platform_get_drvdata(pdev);
  
   if (!sfb-variant.has_clksel)
 - clk_disable(sfb-lcd_clk);
 + clk_disable_unprepare(sfb-lcd_clk);
  
 - clk_disable(sfb-bus_clk);
 + clk_disable_unprepare(sfb-bus_clk);
  
   return 0;
  }
 @@ -1653,10 +1653,10 @@ static int s3c_fb_runtime_resume(struct device *dev)
   struct s3c_fb *sfb = platform_get_drvdata(pdev);
   struct s3c_fb_platdata *pd = sfb-pdata;
  
 - clk_enable(sfb-bus_clk);
 + clk_prepare_enable(sfb-bus_clk);
  
   if (!sfb-variant.has_clksel)
 - clk_enable(sfb-lcd_clk);
 + clk_prepare_enable(sfb-lcd_clk);
  
   /* setup gpio and output polarity controls */
   pd-setup_gpio();

--
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 00/16] pinctrl: samsung: Usability and extensibiltiy improvements

2012-10-10 Thread Tomasz Figa
On Wednesday 10 of October 2012 09:46:28 Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  This patch series is a work on improving usability and extensibiltiy of
  the 
  pinctrl-samsung driver. It consists of three main parts:
   - moving SoC-specific data to device tree
   - converting the driver to use one GPIO chip and one IRQ domain per
   pin bank - introducing generic wake-up interrupt capability
   description
 
 So can you prepare a patch series which does all but the first
 bullet to begin with, and a SoC-and register offset patch
 on top of that as a separate series, because it is controversial?
 
 I don't like that these two things are mingled together like this
 in an all-or-nothing manner.
 
 So I'm OK with a patch series for bulle (2) and (3) but not (1).
 
 And I'd like to have Thomas A:s ACK on the series too.
 

I have managed to rework the changes to drop (1). I will send next version 
of patches tomorrow. It would be nice to have them merged for 3.7, as they 
are rather important for further work.

Moving data from the driver to device tree is not as important, so it might 
be discussed later.

Best regards,
-- 
Tomasz Figa
Samsung Poland RD Center

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


Re: [PATCH v2 0/6] ARM: EXYNOS: Add secure firmware support

2012-10-10 Thread Kyungmin Park
Hi Arnd or Olof,

Can you pick up for v3.7?

To Tomasz,
Can you rebase it on the latest arm-soc tree?

Thank you,
Kyungmin Park

On Tue, Oct 2, 2012 at 6:13 PM, Tomasz Figa t.f...@samsung.com wrote:
 Hi,

 On Monday 24 of September 2012 16:28:27 Tomasz Figa wrote:
 Some Exynos-based boards are running with secure firmware running in
 TrustZone secure world, which changes the way some things have to be
 initialized.

 This series adds support for specifying firmware operations, implements
 some firmware operations for Exynos secure firmware and adds a method of
 enabling secure firmware operations on Exynos-based boards through board
 file and device tree.

 Changes since v1
 http://thread.gmane.org/gmane.linux.kernel.samsung-soc/12583/focus=12820
   - Changed return types of all operations to int
   - Defined all operations to return 0 on success, -ENOSYS when not
 implemented or appropriate error code on error

 Tomasz Figa (6):
   ARM: Add interface for registering and calling firmware-specific
 operations
   ARM: EXYNOS: Add support for secure monitor calls
   ARM: EXYNOS: Add support for secondary CPU bring-up on Exynos4412
   ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
   ARM: EXYNOS: Add support for Exynos secure firmware
   ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up

  .../devicetree/bindings/arm/samsung-boards.txt |  8 
  arch/arm/common/Makefile   |  2 +
  arch/arm/common/firmware.c | 18 
  arch/arm/include/asm/firmware.h| 31 +
  arch/arm/mach-exynos/Makefile  |  6 +++
  arch/arm/mach-exynos/common.c  | 34 ++
  arch/arm/mach-exynos/common.h  |  2 +
  arch/arm/mach-exynos/exynos-smc.S  | 22 +
  arch/arm/mach-exynos/firmware.c| 54
 ++ arch/arm/mach-exynos/include/mach/map.h
  |  3 ++
  arch/arm/mach-exynos/mach-exynos4-dt.c |  1 +
  arch/arm/mach-exynos/platsmp.c | 36 ---
  arch/arm/mach-exynos/smc.h | 31 +
  arch/arm/plat-samsung/include/plat/map-s5p.h   |  1 +
  14 files changed, 243 insertions(+), 6 deletions(-)
  create mode 100644 arch/arm/common/firmware.c
  create mode 100644 arch/arm/include/asm/firmware.h
  create mode 100644 arch/arm/mach-exynos/exynos-smc.S
  create mode 100644 arch/arm/mach-exynos/firmware.c
  create mode 100644 arch/arm/mach-exynos/smc.h

 Any comments, nitpicks or acks on this patchset?

 Best regards,
 --
 Tomasz Figa
 Samsung Poland RD Center

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


[RFC] Samsung SoC camera DT bindings

2012-10-10 Thread Sylwester Nawrocki
Hi All,

The following is a brief description of Samsung SoC architecture from the 
camera point of view and a corresponding device tree structure. It is based 
on the media devices DT bindings design from Guennadi [1]. I  incorporated
some changes proposed during reviews (e.g. s/link/endpoint). It seems the 
common media bindings are more or less settled now and the discussions are 
mostly about implementation of the common parsers and core code.

This RFC is just about an example of the media bindings for fairly complex
SoC architecture. I would be happy to get any feedback on that, while I'm
about to start adding support for this at the driver.

In Samsung SoCs there are multiple capture interfaces that can be attached
to each physical camera port, either parallel video bus or serial MIPI CSI-2.

  c02+-+
  c01 |   |   |  | |
--o-+  CAM_IF.0   |-- memory
+-+  ++   |   |   |  | |
| sensor A (MIPI CSI2)|  | CSI-RX |   |   |   |  +-+
+-+-++--|---o-  |
  |   |   o- from ISP
+--+  |   |   |
| sensor B (parallel)  |  |   |   |
+--+-o-  |   |
  |   |   |  +-+
  c11 |   | c12  | |
--o-+  CAM_IF.1   |-- memory
  |   |   |  | |
  |   |   |  +-+
  .   .   .
  |   |   |  +-+
  c21 |   |c22|  | |  to ISP 
--o-+  CAM_IF_L.0 |-- (SoC
  |   |   |  | |  internal
 +-+  data bus)

As in the above figure, each external sensor can be connected to any of the 
CAM_IFs at run-time. It's also possible to connect two CAM_IFs to a single 
sensor in parallel. CSI-RX devices and parallel video bus port are connected 
to CAM_IF.N devices internally through some sort of crossbar interconnect.

On some SoCs there is also an ISP, which can use one of the two limited 
capture interfaces (CAM_IF_L.0) as front-ends and return video data to
a CAM_IF.N that acts as a DMA engine. Following configurations are possible:

sensor A - mipi-csi2 slave (CSI-RX) - CAM_IF.? - memory
sensor B - CAM_IF.? - memory

sensor A - mipi-csi2 slave (CSI-RX) - CAM_IF_LITE.? - memory
sensor B - CAM_IF_LITE.? - memory

sensor B - CAM_IF_LITE.? - ISP - CAM_IF.? - memory
sensor A - mipi-csi2 slave (CSI-RX) - CAM_IF_LITE.? - ISP - CAM_IF.? 
- memory

Describing all these possible links by our port/endpoint convention would
result in unnecessarily complex structure. These SoC's internal data routing
could well be coded in the driver, depending on the available hardware 
entities and based only on the compatible property. 

On the other hand it would be useful to specify certain initial active link 
configurations, so the device is in known and valid state after the driver
has initialized. I chose to specify only those default internal SoC links
in DT, leaving sorting out all possible routes for the driver.

The below device tree structure contains two camera sensors controlled over
an I2C bus, where m5mols is connected to the SoC through MIPI CSI-2 bus 
and s5k5bafx is on a parallel video bus. There are following default links
specified there:
  1) m5mols - csis0 - fimc0,
  2) s5k4bafx - fimc1.

Any comments and suggestions are welcome.

/*===*/

   /* Aliases for assigning platform entity indexes at the drivers */
aliases {
csis0 = csis_0;
csis1 = csis_1;
fimc0 = fimc_0;
fimc1 = fimc_1;
fimc2 = fimc_2;
fimc3 = fimc_3;
fimc_lite0 = fimc_lite_0;
fimc_lite1 = fimc_lite_1;
};

i2c0: i2c@0xfff2 {
...
/* Parallel bus IF sensor */
s5k5bafx: sensor@0x21 {
compatible = samsung,s5k5bafx;
reg = 0x21;
vddio-supply = regulator1;
vddcore-supply = regulator2;

clock-frequency = 2000;
clocks = mclk0;
clock-names = mclk;

port {
s5k5bafx_1: endpoint {
remote-endpoint = cam_a_endpoint;
bus-width = 8;
hsync-active = 0;
hsync-active = 1;
pclk-sample = 1;
};
};
};

/* MIPI CSI-2 bus IF sensor */
m5mols: 

Re: [PATCH 5/6] ARM: EXYNOS: Add support for Exynos secure firmware

2012-10-10 Thread Olof Johansson
Hi,

On Mon, Sep 24, 2012 at 04:28:32PM +0200, Tomasz Figa wrote:
 Some Exynos-based boards contain secure firmware and must use firmware
 operations to set up some hardware.
 
 This patch adds firmware operations for Exynos secure firmware and a way
 for board code and device tree to specify that they must be used.
 
 Example of use:
 
 In board code:
 
   ...MACHINE_START(...)
   /* ... */
   .init_early = exynos_firmware_init,
   /* ... */
   MACHINE_END
 
 In device tree:
 
   / {
   /* ... */
 
   firmware {
   compatible = samsung,secure-firmware;
   };
 
   /* ... */
   };
 
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 Signed-off-by: Tomasz Figa t.f...@samsung.com
 ---
  .../devicetree/bindings/arm/samsung-boards.txt |  8 
  arch/arm/mach-exynos/Makefile  |  1 +
  arch/arm/mach-exynos/common.h  |  2 +
  arch/arm/mach-exynos/firmware.c| 54 
 ++
  arch/arm/mach-exynos/mach-exynos4-dt.c |  1 +
  5 files changed, 66 insertions(+)
  create mode 100644 arch/arm/mach-exynos/firmware.c
 
 diff --git a/Documentation/devicetree/bindings/arm/samsung-boards.txt 
 b/Documentation/devicetree/bindings/arm/samsung-boards.txt
 index 0bf68be..f447059 100644
 --- a/Documentation/devicetree/bindings/arm/samsung-boards.txt
 +++ b/Documentation/devicetree/bindings/arm/samsung-boards.txt
 @@ -6,3 +6,11 @@ Required root node properties:
  - compatible = should be one or more of the following.
  (a) samsung,smdkv310 - for Samsung's SMDKV310 eval board.
  (b) samsung,exynos4210  - for boards based on Exynos4210 SoC.
 +
 +Optional:
 +- firmware node, specifying presence and type of secure firmware, 
 currently
 +supported value of compatible property is samsung,secure-firmware:
 +
 + firmware {
 + compatible = samsung,secure-firmware;
 + };

If you require the binding to specify the memory area, then you at least allow
for future work to move to a dynamic mapping without updating the binding and
all device trees. So, please do that even if the code is hardcoded to the
static address today.

For extra credit, make sure that the reg property is matching the static mapping
when you setup your firmware interface on your platform.


[...]

 +static int exynos_cpu_boot_reg(int cpu, void __iomem **ptr)
 +{
 + *ptr = S5P_VA_SYSRAM_NS + 0x1c + 4*cpu;
 + return 0;
 +}

It would be nice to get a memory map for the SMC area in documentation
somewhere, but that can be done separately later.


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


Re: [PATCH 6/6] ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up

2012-10-10 Thread Olof Johansson
On Mon, Sep 24, 2012 at 04:28:33PM +0200, Tomasz Figa wrote:
 Boards using secure firmware must use different CPU boot registers and
 call secure firmware to boot the CPU.
 
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 Signed-off-by: Tomasz Figa t.f...@samsung.com
 ---
  arch/arm/mach-exynos/platsmp.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
 index a7f4031..4a18250 100644
 --- a/arch/arm/mach-exynos/platsmp.c
 +++ b/arch/arm/mach-exynos/platsmp.c
 @@ -25,6 +25,7 @@
  #include asm/hardware/gic.h
  #include asm/smp_plat.h
  #include asm/smp_scu.h
 +#include asm/firmware.h
  
  #include mach/hardware.h
  #include mach/regs-clock.h
 @@ -44,6 +45,8 @@ static inline void __iomem *cpu_boot_reg_base(void)
  static inline void __iomem *cpu_boot_reg(int cpu)
  {
   void __iomem *boot_reg;
 + if (!call_firmware_op(cpu_boot_reg, cpu, boot_reg))
 + return boot_reg;

Nit: new lines between variable declaration and code.


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


Re: [PATCH v2 0/6] ARM: EXYNOS: Add secure firmware support

2012-10-10 Thread Olof Johansson
On Mon, Sep 24, 2012 at 04:28:27PM +0200, Tomasz Figa wrote:
 Some Exynos-based boards are running with secure firmware running in
 TrustZone secure world, which changes the way some things have to be
 initialized.
 
 This series adds support for specifying firmware operations, implements
 some firmware operations for Exynos secure firmware and adds a method of
 enabling secure firmware operations on Exynos-based boards through board
 file and device tree.
 
 Changes since v1
 http://thread.gmane.org/gmane.linux.kernel.samsung-soc/12583/focus=12820
   - Changed return types of all operations to int
   - Defined all operations to return 0 on success, -ENOSYS when not
 implemented or appropriate error code on error
 
 Tomasz Figa (6):
   ARM: Add interface for registering and calling firmware-specific
 operations
   ARM: EXYNOS: Add support for secure monitor calls
   ARM: EXYNOS: Add support for secondary CPU bring-up on Exynos4412
   ARM: EXYNOS: Add IO mapping for non-secure SYSRAM.
   ARM: EXYNOS: Add support for Exynos secure firmware
   ARM: EXYNOS: Add secure firmware support to secondary CPU bring-up

Besides the open comment on the device-tree binding, the other patches are:

Reviewed-by: Olof Johansson o...@lixom.net

--
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 0/6] ARM: EXYNOS: Add secure firmware support

2012-10-10 Thread Olof Johansson
Hi,

On Thu, Oct 11, 2012 at 12:35:54AM +0900, Kyungmin Park wrote:
 Hi Arnd or Olof,
 
 Can you pick up for v3.7?
 
 To Tomasz,
 Can you rebase it on the latest arm-soc tree?

This code should have been in arm-soc by the beginning of the merge window
(and in linux-next) to be merged for 3.7, but we will be happy to queue
it up for 3.8 once 3.7-rc1 is out. I have one outstanding comment on
the DT binding but the rest looks OK to me.

Tomasz, please rebase and send this to Kukjin so he can queue it up with other
Samsung code.


Thanks!

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


Re: [PATCH 2/6] ARM: EXYNOS5: DT Support for SATA and SATA PHY

2012-10-10 Thread Olof Johansson
Hi,

On Wed, Oct 10, 2012 at 02:08:31PM +0530, Vasanth Ananthan wrote:

   +
   + sataphy@70 {
 
  sata-phy would be a more conventional name.
 
   + compatible = samsung,i2c-phy;
 
  i2c-phy? Seems like an odd choice of name. What is this device?
 
 
 The SATA physical layer controller is both a platform device and a i2c
 slave device.
 This compatible string is for the i2c client driver. Hence I have used
 i2c-phy here.

Ok, but samsung,i2c-phy is much too generic. Maybe something like
samsung,exynos5-sata-phy  (and rename the MMIO-controlled phy controls
to ...sata-phy-controller below).


   +};
   +
   +sata-phy@1217 {
   +compatible = samsung,exynos-sata-phy;
   +reg = 0x1217 0x1ff;
   +};
 
  Should have binding too. How does this relate to the i2c device above.
 
 
 As mentioned earlier SATA physical layer controller is both a platform
 device and also an i2c slave device.
 This Node is for the SATA physical layer platform device, the previous node
 is for i2c slave device.
 Certain initialization settings done directly and other settings has to be
 done through i2c.

Wow, that's quite awkward. What needs to be done over i2c? I think I have
seen use of SATA without touching the i2c side but it might have been for
a simple setup.

   +
 i2c@12C6 {
 compatible = samsung,s3c2440-i2c;
 reg = 0x12C6 0x100;
   @@ -152,6 +164,13 @@
 #size-cells = 0;
 };
  
   + i2c@121D {
   +compatible = samsung,s3c2440-sataphy-i2c;
 
  Is this a unique i2c controller, or is it just another one like the others
  on
  the chip? If it's the latter, it should use the regular compatible string.
 
 
 Yes, its a unique i2c controller which lacks an interrupt line while others
 are interrupt driven.
 Hence I have used a distinct compatible string for the driver to
 distinguish the controller.

It would be better to just make the i2c bus driver handle the case where there
is no interrupt specifier and just use polling in those cases, especially if
the rest of the device is identical and doesn't need special handling.

As a matter of fact, if that had been done for the hdmi phy, then you could
have done this patch without modifying the driver at all, just device tree
contents. And the same would go for the next time down the road when
a special i2c bus is added.


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


Re: [PATCH 01/16] ARM: dts: exynos4210: Replace legacy GPIO bank nodes with pinctrl bank nodes

2012-10-10 Thread Stephen Warren
On 10/10/2012 01:26 AM, Linus Walleij wrote:
 On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
 
 Seuqential patches from this series introduce SoC-specific data parsing
 from device tree.

 This patch removes legacy GPIO bank nodes from exynos4210.dtsi and
 replaces them with nodes and properties required for these patches.
 
 So to be clear:
 
 +   pinctrl-bank-types {
 +   bank_off: bank-off {
 +   samsung,reg-names = func, dat, pud,
 +   drv, conpdn, pudpdn;
 +   samsung,reg-params = 0x00 4, 0x04 1, 0x08 2,
 +   0x0C 2, 0x10 2, 0x14 2;
 +   };
 
 This is starting to look like a firmware language, I have mixed
 feelings about this. Shall this be read:
 
 Poke 4 into 0x00, poke 1 into 0x04, poke 2 into 0x08 etc?
 
 We really need to discuss this, Grant has already NACK:ed
 such approaches once.

Well, I don't think he NACK'd Tony Lindgren's generic pinctrl driver,
which is doing this exact same thing. I did raise the same point about
Tony's driver when he posted it, but nobody seemed inclined to NACK it
based on that at the time, IIRC...

BTW, the idea here is IIRC to create a generic Samsung pinctrl driver
that works across N different Samsung SoCs, each with different register
layout, without having to encode the register layout into tables in the
kernel.

 If you're still going to do this, it is mandatory
 to NOT use magic hex numbers anymore, because Stephen has
 merged preprocessor support to the DTC compiler so you
 can use #defined macros.
 
 See commit:
 cd296721a9645f9f28800a072490fa15458d1fb7

That feature isn't enabled yet. While dtc has been modified to be able
to accept input that's been generated/processed by cpp, there is still
ongoing discussion about how/whether to actually enable *.dts to use
that feature.
--
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 01/16] ARM: dts: exynos4210: Replace legacy GPIO bank nodes with pinctrl bank nodes

2012-10-10 Thread Tony Lindgren
* Stephen Warren swar...@wwwdotorg.org [121010 09:36]:
 On 10/10/2012 01:26 AM, Linus Walleij wrote:
  On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com wrote:
  
  Seuqential patches from this series introduce SoC-specific data parsing
  from device tree.
 
  This patch removes legacy GPIO bank nodes from exynos4210.dtsi and
  replaces them with nodes and properties required for these patches.
  
  So to be clear:
  
  +   pinctrl-bank-types {
  +   bank_off: bank-off {
  +   samsung,reg-names = func, dat, pud,
  +   drv, conpdn, pudpdn;
  +   samsung,reg-params = 0x00 4, 0x04 1, 0x08 2,
  +   0x0C 2, 0x10 2, 0x14 
  2;
  +   };
  
  This is starting to look like a firmware language, I have mixed
  feelings about this. Shall this be read:
  
  Poke 4 into 0x00, poke 1 into 0x04, poke 2 into 0x08 etc?
  
  We really need to discuss this, Grant has already NACK:ed
  such approaches once.
 
 Well, I don't think he NACK'd Tony Lindgren's generic pinctrl driver,
 which is doing this exact same thing. I did raise the same point about
 Tony's driver when he posted it, but nobody seemed inclined to NACK it
 based on that at the time, IIRC...

To summarize, using reg value pairs in DT makes sense if the amount
of data is huge. Otherwise we'll be describing indidual hardware bits
as properties in DT, or have to have huge amounts of static data in
the kernel.

Where it does not make sense is if there's a sequence of reads
and writes with test loops in between.. But that's does not look
to be the case here.

The reg value pairs will be readable when the DT preprocessing is
available, and that allows the values to be orred together while
DT properties don't. The alternative is to describe hardware register
bits as DT properties, which is very bloated.

But considering all this.. Are the samsung,reg-names really needed
by the kernel?

The pinctrl named modes actually are more generic from the pinctrl
client driver point of view as you can set up multiple states for
runtime PM.
 
 BTW, the idea here is IIRC to create a generic Samsung pinctrl driver
 that works across N different Samsung SoCs, each with different register
 layout, without having to encode the register layout into tables in the
 kernel.
 
  If you're still going to do this, it is mandatory
  to NOT use magic hex numbers anymore, because Stephen has
  merged preprocessor support to the DTC compiler so you
  can use #defined macros.
  
  See commit:
  cd296721a9645f9f28800a072490fa15458d1fb7
 
 That feature isn't enabled yet. While dtc has been modified to be able
 to accept input that's been generated/processed by cpp, there is still
 ongoing discussion about how/whether to actually enable *.dts to use
 that feature.

Hey finally! That's good news.

Regards,

Tony
--
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 01/16] ARM: dts: exynos4210: Replace legacy GPIO bank nodes with pinctrl bank nodes

2012-10-10 Thread Tomasz Figa
Dnia środa, 10 października 2012 11:12:53 Tony Lindgren pisze:
 * Stephen Warren swar...@wwwdotorg.org [121010 09:36]:
  On 10/10/2012 01:26 AM, Linus Walleij wrote:
   On Mon, Oct 8, 2012 at 10:39 AM, Tomasz Figa t.f...@samsung.com 
wrote:
   Seuqential patches from this series introduce SoC-specific data
   parsing
   from device tree.
   
   This patch removes legacy GPIO bank nodes from exynos4210.dtsi and
   replaces them with nodes and properties required for these patches.
   
   So to be clear:
   +   pinctrl-bank-types {
   +   bank_off: bank-off {
   +   samsung,reg-names = func, dat, pud,
   +   drv, conpdn,
   pudpdn; +   samsung,reg-params = 0x00 4,
   0x04 1, 0x08 2, +  
   0x0C 2, 0x10 2, 0x14 2; +   };
   
   This is starting to look like a firmware language, I have mixed
   feelings about this. Shall this be read:
   
   Poke 4 into 0x00, poke 1 into 0x04, poke 2 into 0x08 etc?
   
   We really need to discuss this, Grant has already NACK:ed
   such approaches once.
  
  Well, I don't think he NACK'd Tony Lindgren's generic pinctrl driver,
  which is doing this exact same thing. I did raise the same point about
  Tony's driver when he posted it, but nobody seemed inclined to NACK it
  based on that at the time, IIRC...
 
 To summarize, using reg value pairs in DT makes sense if the amount
 of data is huge. Otherwise we'll be describing indidual hardware bits
 as properties in DT, or have to have huge amounts of static data in
 the kernel.
 
 Where it does not make sense is if there's a sequence of reads
 and writes with test loops in between.. But that's does not look
 to be the case here.
 
 The reg value pairs will be readable when the DT preprocessing is
 available, and that allows the values to be orred together while
 DT properties don't. The alternative is to describe hardware register
 bits as DT properties, which is very bloated.
 
 But considering all this.. Are the samsung,reg-names really needed
 by the kernel?

They are used to specify which registers are defined in reg-params property 
and in which order. Most of the registers are not mandatory and this is 
needed to be able to specify only those that are present. At least I 
couldn't think of a better solution for this. Do you have some suggestions?
 
Best regards,
Tomasz Figa

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