Re: [PATCH v5 4/7] ASoC: sunxi: Add sun8i I2S driver

Jean-Francois Moine
2016-11-08T10:51:29Z

On Mon, 7 Nov 2016 21:05:05 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:

> Hi,
>
> On Sun, Nov 06, 2016 at 07:02:48PM +0100, Jean-Francois Moine wrote:
> > On Sun, 23 Oct 2016 09:33:16 +0800
> > Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> wrote:
> >
> > > On Fri, Oct 21, 2016 at 4:36 PM, Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org> wrote:
> > > > This patch adds I2S support to sun8i SoCs as the A83T and H3.
> > > >
> > > > Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
> > > > ---
> > > > Note: This driver is closed to the sun4i-i2s except that:
> > > > - it handles the H3
> > >
> > > If it's close to sun4i-i2s, you should probably rework that one to support
> > > the newer SoCs.
> >
> > I started to add the H3 into the sun4i-i2s, but I am blocked with
> > regmap.
> > Many H3 registers are common with the A10, but some of them have more
> > or less fields, the fields may be at different offsets. And, finally,
> > some registers are completely different.
> > This would not raise any problem, except with regmap which is really
> > painful.
>
> That's weird, because regmap's regmap_field should make that much
> easier.

#define field_relaxed(addr, mask, val) \
writel_relaxed((readl_relaxed(addr) & mask) | val, addr)

> > As I may understood, regmap is used to simplify suspend/resume, but, is
> > it useful to save the I2S register on suspend?
> > Practically, I am streaming some tune on my device. I suspend it for
> > any reason. The next morning, I resume it. Are you sure I want to
> > continue to hear the end of the tune?
> >
> > I better think that streaming should be simply stopped on suspend.
>
> You're mistaken. The code in there is for *runtime* suspend, ie when
> the device is no longer used, so that case shouldn't even happen at
> all.
>
> (And real suspend isn't supported anyway)

Is it time to remove this useless code?

> > Then, there is no need to save the playing registers, and, here I am,
> > there is no need to use regmap.
> >
> > May I go this way?
>
> No, please don't. regmap is also providing very useful features, such
> as access to all the registers through debugfs, or tracing. What
> exactly feels painful to you?

When the I/O registers are in memory (that's the case), you may access
them (read and write) thru /dev/mem.
Also, is a register access trace really needed in this driver?

The pain is to define the regmap_config (which registers can be
read/write/volatile and which can be the values the u-boot let us in
the registers at startup time), and the lot of code which is run instead
of simple load/store machine instructions.

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.