Messages in this thread | | | From | Laurent Pinchart <> | Subject | Re: [PATCH 11/27] clocksource: sh_cmt: Add support for multiple channels per device | Date | Mon, 17 Feb 2014 02:48:42 +0100 |
| |
Hi Magnus,
On Monday 17 February 2014 10:41:31 Magnus Damm wrote: > On Mon, Feb 17, 2014 at 3:18 AM, Laurent Pinchart wrote: > > On Saturday 15 February 2014 13:46:54 Thomas Gleixner wrote: > >> On Fri, 14 Feb 2014, Laurent Pinchart wrote: > >> > CMT hardware devices can support multiple channels, with global > >> > registers and per-channel registers. The sh_cmt driver currently models > >> > the hardware with one Linux device per channel. This model makes it > >> > difficult to handle global registers in a clean way. > >> > > >> > Add support for a new model that uses one Linux device per timer with > >> > multiple channels per device. This requires changes to platform data, > >> > add new channel configuration fields. > >> > > >> > Support for the legacy model is kept and will be removed after all > >> > platforms switch to the new model. > >> > > >> > Signed-off-by: Laurent Pinchart > >> > <laurent.pinchart+renesas@ideasonboard.com> > >> > --- > >> > > >> > drivers/clocksource/sh_cmt.c | 299 ++++++++++++++++++++++++++--------- > >> > include/linux/sh_timer.h | 9 ++ > >> > 2 files changed, 239 insertions(+), 69 deletions(-) > >> > > >> > diff --git a/drivers/clocksource/sh_cmt.c > >> > b/drivers/clocksource/sh_cmt.c > >> > index 5280231..8390f0f 100644 > >> > --- a/drivers/clocksource/sh_cmt.c > >> > +++ b/drivers/clocksource/sh_cmt.c
[snip]
> >> > @@ -85,11 +94,15 @@ struct sh_cmt_info { > >> > > >> > struct sh_cmt_channel { > >> > struct sh_cmt_device *cmt; > >> > - unsigned int index; > >> > - void __iomem *base; > >> > + unsigned int index; /* Index in the documentation */ > >> > + unsigned int hwidx; /* Real hardware index */ > >> > + > >> > + void __iomem *iostart; > >> > + void __iomem *ioctrl; > >> > > >> > struct irqaction irqaction; > >> > >> While you are at it, can you please get rid of that irqaction and use > >> request_irq() ? > > > > The driver claims it can't use request_irq() because the function is not > > available for early platform devices. If the situation has changed I'd > > gladly get rid of irqaction. > > With the risk of stating the obvious, this depends on how early the > early platform device stuff is being run. The actual location may not > be the same on SH and ARM for instance. > > On ARM we are doing all we can to initialize these devices as late as > ever possible in the MULTIPLAFORM (DT reference) case. Once we manage > to remove the legacy ARM case then there is one less user of early > platform timers. > > One perhaps reasonable way forward could be to use request_irq() in > case of DT and leave the legacy platform data case to rely on > irqaction.
The whole point of switching from setup_irq() to request_irq() is to simplify the code. Adding request_irq() as an option would go in the opposite direction.
-- Regards,
Laurent Pinchart
| |