RE: [PATCH v6 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

-----Original Message-----
From: Chen Guangyu-B42378
Sent: Monday, August 19, 2013 11:55 AM
To: Bhushan Bharat-R65777
Cc: broonie@kernel.org; lars@metafoo.de; p.zabel@pengutronix.de;
s.hauer@pengutronix.de; mark.rutland@arm.com; devicetree@vger.kernel.org; alsa-
devel@alsa-project.org; swarren@wwwdotorg.org; festevam@gmail.com;
timur@tabi.org; rob.herring@calxeda.com; tomasz.figa@gmail.com;
shawn.guo@linaro.org; linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v6 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

Hi Bhushan,

   I'll revise some as you suggest. Just a few replies here.

On Mon, Aug 19, 2013 at 12:38:11PM +0800, Bhushan Bharat-R65777 wrote:

quoted

quoted

We here suppose the reset bit would be cleared -- "The software
reset will last
8 cycles." from RM, so if this happened to be a failure, the whole
IP module won't be normally working as well.

Also add a comment describing this against why cycle = 1000 is selected.

If it is done in 8 cycles, 1000-cycle will be surely a safe value for it.
As long as it finished in 8 cycles, it would quit anyway. Why against?

I am not against, I am saying why it was not 200 or 50 or 20 etc. I am saying that write a comment saying this much is sufficient as per specification and so keep 1000/etc as preservative.

-Bharat

quoted

quoted

quoted

quoted

+static bool fsl_spdif_volatile_reg(struct device *dev, unsigned
+int reg) {
+	/* Sync all registers after reset */

Where us sync :) ?

The "return true" would do that. For volatile registers, if no "return true"
here, the whole regmap would use the value in cache, while for some
bits we need to trace its true value from the physical registers not from

cache.

quoted

Where will be device registers cached? Do not we program them to be non-

cacheable in core?

regmap has a regcache for all the mapped registers. Set the regsiters as
volatile will allow the driver to sync the regcache with physical memory each
time when using regmap_read/write/update_bits().

But I think I can try to use the regcache_bypass instead.


Thank you,
Nicolin Chen