On Wed, 2010-11-17 at 10:55 +0100, Arnd Bergmann wrote:
> On Tuesday 16 November 2010, Joe Perches wrote:
> > static inline u32 MCDE_channel_path(u32 chnl, u32 fifo, u32 type, u32 ifc,
> > u32 link)
> > {
> > return ((chnl << 16) |
> > (fifo << 12) |
> > (ty
On Tuesday 16 November 2010, Joe Perches wrote:
> static inline u32 MCDE_channel_path(u32 chnl, u32 fifo, u32 type, u32 ifc,
> u32 link)
> {
> return ((chnl << 16) |
> (fifo << 12) |
> (type << 8) |
> (ifc << 4) |
> (link << 0
On Tue, 2010-11-16 at 16:29 +0100, Jimmy RUBIN wrote:
> > > +/* Channel path */
> > > +#define MCDE_CHNLPATH(__chnl, __fifo, __type, __ifc, __link) \
> > > + (((__chnl) << 16) | ((__fifo) << 12) | \
> > > + ((__type) << 8) | ((__ifc) << 4) | ((__link) << 0))
> > > +enum mcde_chnl_path {
> > > + /*
sent out too early...
On Tuesday 16 November 2010, Arnd Bergmann wrote:
> > > This looks a bit like you actually have multiple interrupt lines
> > > multiplexed
> > > through a private interrupt controller. Have you considered making this
> > > controller
> > > a separate device to multiplex the i
On Tuesday 16 November 2010, Jimmy RUBIN wrote:
>
> > Even static variables like these can cause problems. Ideally all of
> > these
> > are referenced through a driver private data structure that is passed
> > around
> > with the device. This way you can trivially support multiple devices if
> > t
Hi,
Thank you Arnd for your comments.
> A "hardware abstraction layer" is generally considered a bad thing,
> you're usually better off not advertising your code as being one.
>
> As a rule, the device driver *is* the hardware abstraction, so you
> should not add another one ;-)
>
> > +static v
On Mon, 2010-11-15 at 10:52 +0100, Jimmy RUBIN wrote:
> > Just trivia:
[]
> > It'd be nice to change to continuous_running
> Continous_running [...]
It was just a spelling comment.
continous
continuous
>
> > > +int mcde_dsi_dcs_write(struct mcde_chnl_state *chnl, u8 cmd, u8*
> > data, int len)
>
Hi Joe,
Thanks for your input.
See comments below.
> Just trivia:
>
> > diff --git a/drivers/video/mcde/mcde_hw.c
> b/drivers/video/mcde/mcde_hw.c
>
> []
>
> > +#define dsi_rfld(__i, __reg, __fld) \
> > + ((dsi_rreg(__i, __reg) & __reg##_##__fld##_MASK) >> \
> > + __reg##_##__fld#
On Wednesday 10 November 2010, Jimmy Rubin wrote:
> This patch adds support for MCDE, Memory-to-display controller
> found in the ST-Ericsson ux500 products.
Hi Jimmy,
I haven't looked at what this device does, but I've tried to do
a review based on coding style and common practices. I hope this
On Wed, 2010-11-10 at 13:04 +0100, Jimmy Rubin wrote:
> This patch adds support for MCDE, Memory-to-display controller
> found in the ST-Ericsson ux500 products.
Just trivia:
> diff --git a/drivers/video/mcde/mcde_hw.c b/drivers/video/mcde/mcde_hw.c
[]
> +#define dsi_rfld(__i, __reg, __fld) \
>
10 matches
Mail list logo
