Re: [PATCH v2 1/3] ARM: Add CLPS711X architecture

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

On 22:56 Mon 15 Oct     , Alexander Shiyan wrote:
> On Mon, 15 Oct 2012 19:54:14 +0200
> Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> 
> > On 20:26 Mon 15 Oct     , Alexander Shiyan wrote:
> > > This patch adds new architecture (CLPS711X) into barebox.
> > > The core-logic functionality of the device is built around an ARM720T
> > > processor running at clock speeds up to 90 MHz.
> > > 
> > > Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ...
> > > +#include <linux/clkdev.h>
> > split the code in 2
> >  - clock
> >  - clocksource in drivers/clocksource
> >  and cs as a driver
> drivers/clocksource is missing in master repository and as far I know we are
> made patches for "master" but not for "next".
so base your code on next
> 
> ...
> > > +	osc = 3686400;
> > > +	ext = 13000000;
> > creaet a clock for this
> This is constant frequencies and it is used only here. Is this really need?
but it's still a clock if the driver need clock info fixed or not he request a
clock as the value can change across soc
> 
> ...
> > > +void clps711x_add_uart(unsigned int id)
> > > +{
> > > +	add_generic_device_res("clps711x_serial", id, NULL, 0, NULL);
> > where is the base address address?
> Base address is not needed because it is impossible to request resource for
> registers for this CPU. Registers is shared for all devices.
> 
> > where is the check on the id?
> Simple check is provided in serial driver by definition, for example:
> SYSFLG(x) ? SYSFLG2 : SYSFLG1, where x is "id".
> But, you are right, I will made one more check for id in "probe".
> 
> -- 
> Alexander Shiyan <shc_work@mail.ru>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox