Re: [PATCH 02/16] pinctrl: exynos: Parse wakeup-eint parameters from DT
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
* 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
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