[PATCH V5 57/63] ST SPEAr3xx: Updating plgpio and emi source to make it compliant with single image strategy

viresh kumar viresh.linux at gmail.com
Sat Feb 19 12:30:37 EST 2011


On 2/19/11, Russell King - ARM Linux <linux at arm.linux.org.uk> wrote:
> On Thu, Jan 20, 2011 at 12:56:35PM +0530, Viresh Kumar wrote:
>> Signed-off-by: Viresh Kumar <viresh.kumar at st.com>
>
> Why aren't the plgpio bits of this in patch 16?
>

Because we made these changes after that single image solution. And
yes we should
have done our homework on this cleanly and merged all this in patch 16
and emi's patch.

>> +static u32 plgpio_enb, plgpio_wdata, plgpio_dir, plgpio_rdata, plgpio_ie,
>> +	   plgpio_mis;
>
> 'int' or 'unsigned int' will do for these.  There's no pressing need for
> these to be exactly 32-bit quantities.
>

Yes.

>>  static int __init plgpio_init(void)
>>  {
>> +	if (machine_is_spear310()) {
>> +		plgpio_enb = SPEAR310_PLGPIO_ENB;
>> +		plgpio_wdata = SPEAR310_PLGPIO_WDATA;
>> +		plgpio_dir = SPEAR310_PLGPIO_DIR;
>> +		plgpio_rdata = SPEAR310_PLGPIO_IE;
>> +		plgpio_ie = SPEAR310_PLGPIO_RDATA;
>> +		plgpio_mis = SPEAR310_PLGPIO_MIS;
>> +	} else if (machine_is_spear320()) {
>> +		plgpio_enb = SPEAR320_PLGPIO_ENB;
>> +		plgpio_wdata = SPEAR320_PLGPIO_WDATA;
>> +		plgpio_dir = SPEAR320_PLGPIO_DIR;
>> +		plgpio_rdata = SPEAR320_PLGPIO_IE;
>> +		plgpio_ie = SPEAR320_PLGPIO_RDATA;
>> +		plgpio_mis = SPEAR320_PLGPIO_MIS;
>> +	} else {
>> +		return 0;
>
> If it's not supported, then what about returning an error instead of
> zero?
>

Yes. Error must be returned.

> Can you supersede this patch and patch 16 in the patch system with a
> single patch which adds PLGPIO support with no further updates.
>
> You may also consider combining the EMI updates with the patch which
> creates EMI support in the first place, which I think is patch 36.
>

Will do this for both plgpio and emi.



More information about the linux-arm-kernel mailing list