Subject: Re: [RFC][PATCH] ibm_newemac: PowerPC 440EP/440GR EMAC PHY clock workaround

From: "Steven A. Falco" <[email-masked]>
Date: Tue, 26 Feb 2008 09:10:06 -0500

>>> +static inline void emac_rx_clk_default(struct emac_instance *dev)
>>> +{
>>> +	if (emac_has_feature(dev, EMAC_FTR_440EP_PHY_CLK_FIX)) {
>>> +		unsigned long flags;
>>> +
>>> +		local_irq_save(flags);
>>> +		mtdcri(SDR0, SDR0_MFR, mfdcri(SDR0, SDR0_MFR) &
>>> +					~(SDR0_MFR_ECS >> dev->cell_index));
>>> +		local_irq_restore(flags);
>>> +	}
>>> +}
>>>       
>> Why did you do local_irq_save in these two functions?  mtdcri already
>> does spin_lock_irqsave...
>>
>> josh
>>     
>
> Oops, this got copy-pasted from the older ibm_emac.
> Thanks,
> Valentine.
> _______________________________________________
When I saw Josh's question, I thought the irq save/restore was there to
make the read-modify-write atomic; i.e. read SDR0_MFR, "AND" out some
bits, then write it back without the possibility of anything else
touching SDR0_MFR.  I'm just starting to get familiar with the kernel,
so if you have a chance, please help educate me.  Does the irq lock in
mtdcri protect the read-modify-write? Or maybe this R-M-W doesn't need
protecting?

    Steve