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

[Jean-Francois Moine]

On Mon, 7 Nov 2016 21:05:05 +0100
Maxime Ripard <maxime.ripard at free-electrons.com> 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 at csie.org> wrote:
> > 
> > > On Fri, Oct 21, 2016 at 4:36 PM, Jean-Francois Moine <moinejf at free.fr> wrote:
> > > > This patch adds I2S support to sun8i SoCs as the A83T and H3.
> > > >
> > > > Signed-off-by: Jean-Francois Moine <moinejf at free.fr>
> > > > ---
> > > > 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/