Hi Yong,
On Tue, Jan 11, 2011 at 02:23:16PM +0800, yong.shen at freescale.com wrote:
SMD and LOCO are newly developed boards of i.MX53. LOCO means "low cost",
which is supposed to be widespread soon.
These two patches seem almost identical. Can't both board support code share
the same file?
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Hi Baruch,
So far they are similar, but as more code and features are added in,
they are going to be different. We had registered different mach id to
distinguish these two boards.
Thanks
Yong
On Tue, Jan 11, 2011 at 3:23 PM, Baruch Siach [off-list ref] wrote:
Hi Yong,
On Tue, Jan 11, 2011 at 02:23:16PM +0800, yong.shen at freescale.com wrote:
quoted
SMD and LOCO are newly developed boards of i.MX53. LOCO means "low cost",
which is supposed to be widespread soon.
These two patches seem almost identical. Can't both board support code share
the same file?
baruch
--
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ~. .~ ? Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
? - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Hi Yong,
On Tue, Jan 11, 2011 at 03:54:43PM +0800, Yong Shen wrote:
So far they are similar, but as more code and features are added in,
they are going to be different. We had registered different mach id to
distinguish these two boards.
Code for two (or more) different machines can be shared in the same file. See
for example the code in arch/arm/mach-at91/board-stamp9g20.c which supports
both taskit PortuxG20 and taskit Stamp9G20, and
arch/arm/mach-at91/board-pcontrol-g20.c which is reusing code from these
boards for a third board (PControl G20).
We should avoid code duplication as much as possible, IMO.
baruch
On Tue, Jan 11, 2011 at 3:23 PM, Baruch Siach [off-list ref] wrote:
quoted
On Tue, Jan 11, 2011 at 02:23:16PM +0800, yong.shen at freescale.com wrote:
quoted
SMD and LOCO are newly developed boards of i.MX53. LOCO means "low cost",
which is supposed to be widespread soon.
These two patches seem almost identical. Can't both board support code share
the same file?
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Code for two (or more) different machines can be shared in the same file. See
for example the code in arch/arm/mach-at91/board-stamp9g20.c which supports
both taskit PortuxG20 and taskit Stamp9G20, and
arch/arm/mach-at91/board-pcontrol-g20.c which is reusing code from these
boards for a third board (PControl G20).
We should avoid code duplication as much as possible, IMO.
I agree the rule you mentioned about: avoid code duplication, and I
also know that one file can hold multiple board definitions. However,
in this case, these two boards were designed for different purpose.
For example, SMD is for smart mobile device, while LOCO is more
generic for multiple usage. As time goes on, the code for each boards
are supposed to be much more different than it is now.
So I still stick on my opinion, hope you can understand.
thanks
Yong
Hi Fabio,
Below is my reply to Baruch, I meant to keep this code here to show
the reset process clearly.
quoted
+ if (ret) {
+ printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
+ return;
+ }
+ gpio_direction_output(SMD_FEC_PHY_RST, 0);
+ gpio_set_value(SMD_FEC_PHY_RST, 0);
This seems to be redundant. gpio_direction_output() has already set the value
to 0.
You are right. But, gpio_set_value is meant to be here to show the
process of fec reset: first pull low and then pull high. And
gpio_direction_output here is for direction configuration although it
has the ability of configure output value.
thanks
Yong
On Wed, Jan 12, 2011 at 10:46:24AM +0800, Yong Shen wrote:
Hi Fabio,
Below is my reply to Baruch, I meant to keep this code here to show
the reset process clearly.
quoted
quoted
+ if (ret) {
+ printk(KERN_ERR"failed to get GPIO_FEC_PHY_RESET: %d\n", ret);
+ return;
+ }
+ gpio_direction_output(SMD_FEC_PHY_RST, 0);
+ gpio_set_value(SMD_FEC_PHY_RST, 0);
This seems to be redundant. gpio_direction_output() has already set the value
to 0.
You are right. But, gpio_set_value is meant to be here to show the
process of fec reset: first pull low and then pull high. And
gpio_direction_output here is for direction configuration although it
has the ability of configure output value.
If you care about it: I'd vote for removing the call to gpio_set_value,
too.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |