Re: [U-Boot] [PATCH v5] Exynos5: Pinmux: Add fdt for pinmux
Hi Simon,
Hi Akshay,
On Thu, Mar 7, 2013 at 6:09 AM, Akshay Saraswat akshay.s@samsung.com wrote:
This patch adds fdt nodes for peripherals which require pin muxing and configuration. Device tree bindings for pinctrl are kept same as required for Linux. Existing pinmux code modified to retrieve gpio range and function related info from fdt.
Depends-on: [U-Boot] [PATCH 0/4 V3] EXYNOS5: Add GPIO numbering feature URL: http://lists.denx.de/pipermail/u-boot/2013-February/146151.html
Signed-off-by: Akshay Saraswat akshay.s@samsung.com
Changes since v1: - Device tree bindings changed to linux style. - Added documentation for samsung pinctrl.
Changes since v2: - Rebased as per new version of GPIO numbering patch-set.
Changes since v3: - Added comments to reduce ambiguity and increase readability. - Fixed few other nits.
Changes since v4: - Added support for reading peripheral pinctrl subnode names from preipheral's node instead of hard coding.
Well I have to say this is looking really nice. From what I can tell you are using the Linux binding for samsung,pinctrl-names - please can you add in that binding information to your binding file - I can't see it there?
"samsung,pinctrl-names" is already mentioned in example 3 of doc/device-tree-bindings/samsung-pinctrl.txt. samsung-pinctrl.txt in u-boot and kernel are same except the name exynos5 replaced exynos4210. Please tell me, if I misunderstood the requirement.
The interesting thing is that you should at some point (further patch) be able to remove the alias stuff and have the driver's pass their node offset into this function. Then we can drop the peripheral IDs altogether perhaps?
I think for now we should keep PERIPH_ID's because same pinmux.c and functions are being used by exynos4 and exynos4x12. Since, I thought we should not break support for these SoC's, I kept node retrieval in Pinmux itself instead of drivers or smdk5250.c. Please suggest.
arch/arm/cpu/armv7/exynos/pinmux.c | 357 +++++++------- arch/arm/dts/exynos5250-pinctrl.dtsi | 675 +++++++++++++++++++++++++++ arch/arm/dts/exynos5250.dtsi | 92 ++++ board/samsung/dts/exynos5250-smdk5250.dts | 11 + doc/device-tree-bindings/samsung-pinctrl.txt | 253 ++++++++++ include/fdtdec.h | 4 + lib/fdtdec.c | 4 + 7 files changed, 1231 insertions(+), 165 deletions(-) create mode 100644 arch/arm/dts/exynos5250-pinctrl.dtsi create mode 100644 doc/device-tree-bindings/samsung-pinctrl.txt
[..]
diff --git a/include/fdtdec.h b/include/fdtdec.h index 77f244f..26692e5 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -81,6 +81,10 @@ enum fdt_compat_id { COMPAT_SAMSUNG_EXYNOS_EHCI, /* Exynos EHCI controller */ COMPAT_SAMSUNG_EXYNOS_USB_PHY, /* Exynos phy controller for usb2.0 */ COMPAT_MAXIM_MAX77686_PMIC, /* MAX77686 PMIC */
COMPAT_SAMSUNG_EXYNOS_UART, /* Exynos serial */COMPAT_SAMSUNG_EXYNOS_MSHC, /* Exynos MMC controller */COMPAT_SAMSUNG_EXYNOS_I2S, /* Exynos MMC controller */COMPAT_SAMSUNG_PINCTRL, /* PINCTRL */ COMPAT_COUNT,}; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 3ae348d..88dd669 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -56,6 +56,10 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(SAMSUNG_EXYNOS_EHCI, "samsung,exynos-ehci"), COMPAT(SAMSUNG_EXYNOS_USB_PHY, "samsung,exynos-usb-phy"), COMPAT(MAXIM_MAX77686_PMIC, "maxim,max77686_pmic"),
COMPAT(SAMSUNG_EXYNOS_UART, "samsung,exynos-uart"),COMPAT(SAMSUNG_EXYNOS_MSHC, "samsung,exynos-mshc"),COMPAT(SAMSUNG_EXYNOS_I2S, "samsung,exynos-i2s"),COMPAT(SAMSUNG_PINCTRL, "samsung,pinctrl"),Do these match the kernel names?
In kernel these are mentioned as ""samsung,exynos5250-dw-mshc"" & "samsung,exynos4210-uart". I wanted to keep it consistent with others like "samsung,exynos-ehci", "samsung,exynos-spi" etc., hence, made "samsung,exynos-mshc" & "samsung,exynos-uart". Shall I change it ?
};
Regards, Akshay Saraswat
Hi Akshay,
On Fri, Mar 8, 2013 at 4:08 AM, Akshay Saraswat akshay.s@samsung.comwrote:
Hi Simon,
Hi Akshay,
On Thu, Mar 7, 2013 at 6:09 AM, Akshay Saraswat akshay.s@samsung.com
wrote:
This patch adds fdt nodes for peripherals which require pin muxing and configuration. Device tree bindings for pinctrl are kept same as required for Linux. Existing pinmux code modified to retrieve gpio range and function related info from fdt.
Depends-on: [U-Boot] [PATCH 0/4 V3] EXYNOS5: Add GPIO numbering feature URL: http://lists.denx.de/pipermail/u-boot/2013-February/146151.html
Signed-off-by: Akshay Saraswat akshay.s@samsung.com
Changes since v1: - Device tree bindings changed to linux style. - Added documentation for samsung pinctrl.
Changes since v2: - Rebased as per new version of GPIO numbering patch-set.
Changes since v3: - Added comments to reduce ambiguity and increase readability. - Fixed few other nits.
Changes since v4: - Added support for reading peripheral pinctrl subnode names
from preipheral's node instead of hard coding.
Well I have to say this is looking really nice. From what I can tell you are using the Linux binding for samsung,pinctrl-names - please can you add in that binding information to your binding file - I can't see it there?
"samsung,pinctrl-names" is already mentioned in example 3 of doc/device-tree-bindings/samsung-pinctrl.txt. samsung-pinctrl.txt in u-boot and kernel are same except the name exynos5 replaced exynos4210. Please tell me, if I misunderstood the requirement.
I don't see 'samsung,pinctrl-names' in example 3, but also, is it possible to mention this in the binding somehow?
The names should match the kernel if possible.
The interesting thing is that you should at some point (further patch) be able to remove the alias stuff and have the driver's pass their node offset into this function. Then we can drop the peripheral IDs altogether perhaps?
I think for now we should keep PERIPH_ID's because same pinmux.c and functions are being used by exynos4 and exynos4x12. Since, I thought we should not break support for these SoC's, I kept node retrieval in Pinmux itself instead of drivers or smdk5250.c. Please suggest.
That's fine. I suspect perhaps driver could pass their node offset, but let's worry about that another time.
arch/arm/cpu/armv7/exynos/pinmux.c | 357 +++++++------- arch/arm/dts/exynos5250-pinctrl.dtsi | 675
+++++++++++++++++++++++++++
arch/arm/dts/exynos5250.dtsi | 92 ++++ board/samsung/dts/exynos5250-smdk5250.dts | 11 + doc/device-tree-bindings/samsung-pinctrl.txt | 253 ++++++++++ include/fdtdec.h | 4 + lib/fdtdec.c | 4 + 7 files changed, 1231 insertions(+), 165 deletions(-) create mode 100644 arch/arm/dts/exynos5250-pinctrl.dtsi create mode 100644 doc/device-tree-bindings/samsung-pinctrl.txt
[..]
diff --git a/include/fdtdec.h b/include/fdtdec.h index 77f244f..26692e5 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -81,6 +81,10 @@ enum fdt_compat_id { COMPAT_SAMSUNG_EXYNOS_EHCI, /* Exynos EHCI controller */ COMPAT_SAMSUNG_EXYNOS_USB_PHY, /* Exynos phy controller for
usb2.0 */
COMPAT_MAXIM_MAX77686_PMIC, /* MAX77686 PMIC */
COMPAT_SAMSUNG_EXYNOS_UART, /* Exynos serial */COMPAT_SAMSUNG_EXYNOS_MSHC, /* Exynos MMC controller */COMPAT_SAMSUNG_EXYNOS_I2S, /* Exynos MMC controller */
This comment should be I2C.
COMPAT_SAMSUNG_PINCTRL, /* PINCTRL */ COMPAT_COUNT,}; diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 3ae348d..88dd669 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -56,6 +56,10 @@ static const char * const compat_names[COMPAT_COUNT]
= {
COMPAT(SAMSUNG_EXYNOS_EHCI, "samsung,exynos-ehci"), COMPAT(SAMSUNG_EXYNOS_USB_PHY, "samsung,exynos-usb-phy"), COMPAT(MAXIM_MAX77686_PMIC, "maxim,max77686_pmic"),
COMPAT(SAMSUNG_EXYNOS_UART, "samsung,exynos-uart"),COMPAT(SAMSUNG_EXYNOS_MSHC, "samsung,exynos-mshc"),COMPAT(SAMSUNG_EXYNOS_I2S, "samsung,exynos-i2s"),COMPAT(SAMSUNG_PINCTRL, "samsung,pinctrl"),Do these match the kernel names?
In kernel these are mentioned as ""samsung,exynos5250-dw-mshc"" & "samsung,exynos4210-uart". I wanted to keep it consistent with others like "samsung,exynos-ehci", "samsung,exynos-spi" etc., hence, made "samsung,exynos-mshc" & "samsung,exynos-uart". Shall I change it ?
Yes, sorry, but I think we should keep things the same as the kernel. Perhaps that should be a separate patch to sync up the names?
Regards, Simon
};
Regards, Akshay Saraswat
participants (2)
-
Akshay Saraswat -
Simon Glass