Re: shared config registers and locking

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

> > The specific example that made me think about it was some clock control
> > registers on 4xx that the EMAC driver is whacking every now and then (on
> > link state change, no lock contention to be worried about here), though
> > they could be used by other drivers too (or platform code). Since those
> > registers are a bit different from one 4xx to the next too, and since I
> > now have a non-4xx platform that needs to use EMAC and has similar clock
> > control bits I might have to toggle there too, I'm basically ending up
> > with a global spinlock that I can't precisely define better than "global
> > clock control lock" ....
> >
> 
> Do all EMAC implementations have this problem?

No, only some of them. 

> Another approach might be to factor out the modification code only,
> keeping the original locking code for comparation...
> 	static inline modify_dcr(reg, set_mask, reset_mask)
> 	{
> 		unsigned long flags;
> 		local_irq_save(flags);
> 		mtdcr(reg, mfdcr(reg) & ~reset_mask | set_mask ))
> 		local_irq_restore(flags);
> 	}
> and
> 	static inline SDR_MODIFY(...)

Since we need a spinlock here (the whole point of the exercise is to
make it SMP and -rt safe), that means having a global spinlock used by
modify_dcr / SDR_MODIFY which boils down to something not far from my
proposal.

> #if defined(CONFIG_405EP)
> #define EMAC_RX_CLK_TX(idx) modify_dcr(0xf3, 1<< (idx), 0)
> #else
> #define EMAC_RX_CLK_TX(idx) SDR_MODIFY(DCRN_SDR_MFR, (0x08000000 >> idx), 0)
> #endif

The ifdef's are going away in my rework in favor of something like

	if (emac_has_feature(dev, EMAC_FTR_NEED_405EP_CLK_FIX))
		xxxx

So that we can build support for multiple models in one driver if we
want. If we don't, the above if will turn into if (0) / if (1) depending
on what we build for and thus the compiler will do the right thing and
optimize out unused code. That's the approach we use already for CPU and
firmware features and it works well.

Ben.