From mboxrd@z Thu Jan 1 00:00:00 1970 From: Przemyslaw Marczak Date: Mon, 28 Apr 2014 11:47:37 +0200 Subject: [U-Boot] [PATCH v9 0/2] S5P: Exynos: Add GPIO numbering feature In-Reply-To: <1398439905-7576-1-git-send-email-akshay.s@samsung.com> References: <1398439905-7576-1-git-send-email-akshay.s@samsung.com> Message-ID: <535E23B9.1040105@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Akshay, On 04/25/2014 05:31 PM, Akshay Saraswat wrote: > Hi Przemyslaw Marczak, > >> Hello, >> >> On 04/21/2014 04:25 PM, Akshay Saraswat wrote: >>> Used a script to recheck/verify pin mapping and existing mapping >>> appears to be fine, returning correct bank and pin values. >>> Did ./MAKEALL -a arm and found all Exynos/S5P related boards >>> compiled successfully. >>> Couldn't test booting over all of them. >>> Tested U-Boot bootup over SMDK5420, SMDK5250, Snow. >>> Requesting maintaners to please test over affected SoCs/boards. >>> >>> Changes in V2: >>> - Enabled CMD_GPIO as suggested by Simon Glass >>> and supported same for EXYNOS5. >>> Changes in V3: >>> - New patch added to rename S5P GPIO definitions >>> to S5P_GPIO. >>> - GPIO Table added to calculate the base address >>> of input gpio bank. >>> Changes in V4: >>> - To have consistent 0..n-1 GPIO numbering the banks >>> are divided into different parts where ever they >>> have holes in them. >>> - Function and table to support gpio command moved >>> to s5p-gpio driver. >>> - Rebased on latest u-boot-samsung tree. >>> Changes in V5: >>> - Rebased on latest u-boot-samsung tree. >>> - Removed Exynos5 specific code in gpio driver api to >>> get bank. >>> - Added #define HAVE_GENERIC_GPIO in config file >>> to remove conditinal CPU check in gpio driver. >>> Changes in V6: >>> - Isolated config changes in a new patch. >>> - Updated patches with corresponding changes for Exynos 5420. >>> Changes in V7: >>> - Added changes for other SoCs like Exynos 4412, 4210 etc. >>> Changes in V8: >>> - Changed Subject of patch 2/2 to reflect affected SoCs/boards. >>> - Fixed arndale board compile time errors introduced due to >>> patch-set v7. >>> Changes in V9: >>> - Fixed checkpatch errors. >>> - Fixed naming error in exynosxxxx_gpio_data arrays which could >>> be the possible reason behind data abort witnessed over >>> Exynos4 boards. >>> >>> Akshay Saraswat (2): >>> EXYNOS: Add GPIO pin numbering and rename definitions >>> S5P: Exynos: Config: Enable Generic GPIO and CMD configs >>> >>> arch/arm/cpu/armv7/exynos/pinmux.c | 403 +++---- >>> arch/arm/include/asm/arch-exynos/cpu.h | 17 +- >>> arch/arm/include/asm/arch-exynos/gpio.h | 1786 +++++++++++++++++++++++++----- >>> arch/arm/include/asm/arch-s5pc1xx/gpio.h | 941 +++++++++++++--- >>> board/samsung/arndale/arndale.c | 11 +- >>> board/samsung/goni/goni.c | 26 +- >>> board/samsung/smdk5250/exynos5-dt.c | 20 +- >>> board/samsung/smdk5250/smdk5250.c | 19 +- >>> board/samsung/smdk5420/smdk5420.c | 15 +- >>> board/samsung/smdkc100/smdkc100.c | 5 +- >>> board/samsung/smdkv310/smdkv310.c | 17 +- >>> board/samsung/trats/trats.c | 39 +- >>> board/samsung/trats2/trats2.c | 74 +- >>> board/samsung/universal_c210/universal.c | 51 +- >>> drivers/gpio/s5p_gpio.c | 195 +++- >>> include/configs/arndale.h | 1 + >>> include/configs/exynos5-dt.h | 3 + >>> include/configs/origen.h | 1 + >>> include/configs/s5p_goni.h | 5 +- >>> include/configs/s5pc210_universal.h | 17 +- >>> include/configs/smdkc100.h | 1 + >>> include/configs/smdkv310.h | 2 + >>> include/configs/trats.h | 9 +- >>> include/configs/trats2.h | 5 +- >>> 24 files changed, 2806 insertions(+), 857 deletions(-) >>> >> >> I think that you missed my last few comments. >> You are still using exynos_gpio_get() - what for? >> It returns some number and next it's interpreted as >> gpio continuous number - but it isn't continuous. >> It's because each samsung board has now defined "HAVE_GENERIC_GPIO", >> so gpio numbers are interpreted as linear. >> We don't need additional macro since you introduce linear numbering. >> >> Please read my last comments again. > > I have read your comments and replied to it. I am sorry may be I was not clear. > Please look at the patch-set 2/2 again, I am not introducing I just replaced > bank and pin number with sequential pin number. exynos_gpio_get() function was > already there. I can't afford to remove it because I2C driver uses it and > also I don't have boards to test outcomes of such changes. > In opinion it would be better if the maintainer of the board could do these > changes. > >> >> I tried test this on trats2 device but data abort occurs at PMIC init, >> it's probably because exynos_gpio_get(). Please use just gpio numbering >> as you defined in this patchset. >> > > As far as I could understand, exynos_gpio_get still returns what it used to before > this patch because we still extract bank and pin number from linear number to do > the needfull. > But I will definitely try to look for the bug. Thanks for the suggestion. > If you replace every call like: "exynos_gpio_get(AAA, BBB)" with just "BBB" - then this code will work. example for trats2.h: --------------------------------------- exynos_gpio_get(2, EXYNOS4X12_GPIO_X22) needs change to -> EXYNOS4X12_GPIO_X22 --------------------------------------- and one of functions in trats2.c: --------------------------------------- int get_soft_i2c_scl_pin(void) { if (I2C_ADAP_HWNR) - return exynos_gpio_get(2, EXYNOS4X12_GPIO_M21); + return EXYNOS4X12_GPIO_M21; else - return exynos_gpio_get(1, EXYNOS4X12_GPIO_F14); + return EXYNOS4X12_GPIO_F14; } --------------------------------------- I2C will works fine because it uses common gpio calls like "gpio_get_value("BBB")" You don't need to define GPIOs numbers by "exynos_gpio_get()" since you have defined each PIN directly like this: "EXYNOS4X12_GPIO_D00" Even if exynos_gpio_get() returns a proper value code will fail because of this part of code:" (driver/gpio/s5p_gpio.c) #ifdef HAVE_GENERIC_GPIO (defined in every samsung config) struct s5p_gpio_bank *s5p_gpio_get_bank(unsigned int gpio) ... ... and next this part of code: data = get_gpio_data(); count = get_bank_num(); for (i = upto = 0; i < count; i++, upto = data->max_gpio, data++) { debug("i=%d, upto=%d\n", i, upto); if (gpio < data->max_gpio) { (This condition above probably will never met) .. } } return NULL; ... This probably will always return NULL because "gpio" returned by exynos_gpio_get() is a very big number(part mask -> 0xff000000). So if you remove exynos_gpio_get() - it should works. And also please check .dts files. Each gpio number from .dts you should decode by follow current gpio code and it should be replaced by a proper PIN number from gpio enum. Gpio coding mask: 0x000000ff - pin number 0x00ffff00 - bank offset 0xff000000 - part number Akshay, do you now understand what I mean? >> >> Thank you >> -- >> Przemyslaw Marczak >> Samsung R&D Institute Poland >> Samsung Electronics >> p.marczak at samsung.com > > Regards, > Akshay Saraswat > Thank you -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marczak at samsung.com