-----Original Message----- From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] Sent: 29 May 2012 16:59 To: Prafulla Wadaskar Cc: holger.brunck@keymile.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore functions
-----Original Message----- From: Valentin Longchamp [mailto:valentin.longchamp@keymile.com] Sent: 29 May 2012 14:15 To: Prafulla Wadaskar Cc: holger.brunck@keymile.com; u-boot@lists.denx.de Subject: Re: [PATCH 1/3] kirkwood: add kirkwood_mpp_save/restore functions
...snip...
+void kirkwood_mpp_save(void)
This should be void kirkwood_mpp_save(unsigned int *mpp_ctrl, int len)
Here we save _all_ mpp registers, with direct access to the
registers.
With your proposed solution, I would save it in a dynamically allocated table
of
size len.
That's fine for me, but I would then need to export MPP_NR_REGS, because that is what I would pass as len arg, is that OK ?
I think in your case you need configuration of 4 MPPs, i.e. from
MPP6 to MPP11, so you may declare array of length 7 and backup and restore the same MPPs using len = 6.
No Need to backup and restore all MPPs.
Sorry, but this is exactly what you did in the kirkwood_mpp_conf function, you read and write all the registers every time you need to change only one pin, I took it from there for consistency:
4efb77d4 cpu/arm926ejs/kirkwood/mpp.c (Prafulla Wadaskar
2009-06-20 11:01:53 +0200 76) for (i = 0; i < MPP_NR_REGS; i++) {
4efb77d4 cpu/arm926ejs/kirkwood/mpp.c (Prafulla Wadaskar
2009-06-20 11:01:53 +0200 77) writel(mpp_ctrl[i], MPP_CTRL(i));
4efb77d4 cpu/arm926ejs/kirkwood/mpp.c (Prafulla Wadaskar
2009-06-20 11:01:53 +0200 78) debug(" %08x", mpp_ctrl[i]);
4efb77d4 cpu/arm926ejs/kirkwood/mpp.c (Prafulla Wadaskar
2009-06-20 11:01:53 +0200 79) }
And the way you did it is logical, if it was not done like that, a lot of reading/masking/rewriting would be needed, and this for every single pin, so it would be much less efficient than just reading all the regs and write them all back.
Yes, but the idea is that mpp_config function is called just once during initialization, we have exposed a simple array to do this configuration.
but save/restore will be called very frequently, may be for each SPI transaction. So there must be some optimisation. Secondly, we should only tweak only needed MPPs in run time, why all? It may lead to some other side effects (I don't know).
Regards.. Prafulla . . .