Re: [PATCHv9 1/3] mfd: altera: Add Altera SDRAM Controller

From: Thor Thayer
Date: Mon Aug 04 2014 - 12:02:44 EST



On 08/02/2014 12:08 PM, Steffen Trumtrar wrote:
Hi!

On Fri, Aug 01, 2014 at 05:27:57PM -0500, Thor Thayer wrote:
On 08/01/2014 03:13 AM, Lee Jones wrote:
On Thu, 31 Jul 2014, Thor Thayer wrote:
On 07/31/2014 03:26 AM, Lee Jones wrote:
On Wed, 30 Jul 2014, tthayer@xxxxxxxxxxxxxxxxxxxxx wrote:
[...]

+u32 altera_sdr_readl(struct altera_sdr *sdr, u32 reg_offset)
+{
+ return readl(sdr->reg_base + reg_offset);
+}
+EXPORT_SYMBOL_GPL(altera_sdr_readl);
+
+void altera_sdr_writel(struct altera_sdr *sdr, u32 reg_offset, u32 value)
+{
+ writel(value, sdr->reg_base + reg_offset);
+}
+EXPORT_SYMBOL_GPL(altera_sdr_writel);
We'd prefer to use syscon and that is what we started with. If you'd
like to be our advocate, I will return to that because it was pretty
clean. My primary concern is to get it upstreamed and if it is MFD
then I'll make the changes.

Here are the threads.
http://marc.info/?l=linux-kernel&m=140128791902800&w=2
The conclusion of this thread was syscon for offset 0x0, no ?!
And if you decide to have new writel/readl functions, I'd prefer if you don't
change the order of parameters just because. That always weirds me out, when
there are vendorname_writel functions, that only change the API of writel and
nothing else (not exactly the case here).
Hi Steffen,

Yes, I see your point on the order of parameters. i will change it.

As for the syscon only at offset 0, I submitted that in version 7 & 8. It wasn't as clean as the version using syscon for the entire range. It could also require changes to the SDRAM controller binding as things are added in the future. My understanding is the bindings should not change significantly and changes are frowned upon.

After going through this exercise in a couple of different ways, I'm leaning toward using syscon for the entire SDRAM controller register range.

Is there a good reason not to use syscon on the entire register range?

Thanks,

Thor

Regards,
Steffen


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/