[U-Boot] [PATCH v6 1/4] EXYNOS5: Add gpio pin numbering feature

[Akshay Saraswat]

Hi Lukasz,

>Hi Akshay,
>
>I'm not Samsung tree maintainer, but by chance I've come across those
>patches and...
>
>First question - why have you omitted u-boot-samsung tree maintainer?
>I've added Minkyu to CC.
>

Minkyu has an email ID "promsoft at gmail.com" and I added that in CC.
Probably you don't know this email id :-)

>
>Also in the cover letter you claim that this patch was "build tested"
>for Exynos4 based boards. Why didn't you add at least one maintainer of
>those boards to CC?
>

In cover letter I have not mentioned anywhere that I have built or tested
these patches over Exynos4. Patch 2/4 says "Build tested" because Rajeshwari
did build images for Exynos4 boards and that was successfull but nobody tested
booting those images.
I do not possess any Exynos4 board. These patches are meant for Exynos5 only.
But Yes, there are compiler errors introduced for smkc100 because of this new
patch-set and I will fix them in the next patch-set.

>
>> +
>> +/* A list of valid GPIO numbers for the asm-generic/gpio.h interface
>> */ +enum exynos5_gpio_pin {
>> +	/* GPIO_PART1_STARTS */
>> +	EXYNOS5_GPIO_A00,	/* 0 */
>> +	EXYNOS5_GPIO_A01,
>> +	EXYNOS5_GPIO_A02,
>> +	EXYNOS5_GPIO_A03,
>> +	EXYNOS5_GPIO_A04,
>
>According to the patch description, you had a compilation error when
>were adding the support for Exynos 5250 and 5420. Why you fix the
>problem by rewriting the whole framework?
>

This framework is not intended to fix compiler warnings or errors but to make
GPIO numbering easy to remember and sequential, without any holes in between.

>
>IN the patch 2/4 you have:
>
>-		gpio_cfg_pin(start + i, GPIO_FUNC(0x2));
>-		gpio_set_pull(start + i, GPIO_PULL_NONE);
>-		gpio_set_drv(start + i, GPIO_DRV_4X);
>+		gpio_cfg_pin(start + i, S5P_GPIO_FUNC(0x2));
>+		gpio_set_pull(start + i, S5P_GPIO_PULL_NONE);
>+		gpio_set_drv(start + i, S5P_GPIO_DRV_4X);
>
>What is the rationale to change the name to S5P_GPIO and not stick to
>GPIO_FUNC? In which way gpios for Exynos5 are different than for
>Exynos4? Cannot we finally reuse the Exynos 4 and 5 code?
>

We have enum member GPIO_INPUT in common/cmd_gpio.c and GPIO_INPUT define
in arch-exynos/gpio.h. To remove such conflicts we renamed all s5p defines
from "GPIO_*" to "S5P_GPIO_*".
We are using the same s5p_gpio.c for both Exynos4 and 5 as far as I know.
I dont get the exact issue here. Do you want me to remove "S5P_". Is that it ?

>
>With the same patch:
>
>-  	case PERIPH_ID_UART1:
>-		bank = &gpio1->d0;
>-		start = 0;
>+		start = EXYNOS5_GPIO_D00;
>
>What is wrong with specifying the bank field? 
>Why your gpio command cannot use the bank approach?
>

Ultimately we are using banks and pin_nums specific to the bank only
after we extract exact bank from the sequential pin_num.

>
>And one more question: Is this work compliant with new driver model,
>which will be accepted at the merge window after the v2014.04 release?
>
>
>If not, then there is no point to review this code, since GPIO would
>need to be adjusted to use this framework.
>

Please explain more. I don't get this as well :-)

>--
>Best regards,
>
>Lukasz Majewski
>
>Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
>

Regards,
Akshay Saraswat