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

[Josh Boyer <jwboyer@gmail.com>]

On Fri, 22 Feb 2008 22:28:17 +0300
Valentine Barshak <vbarshak@ru.mvista.com> wrote:

> This patch adds ibm_newemac phy clock workaround for 440EP/440GR emacs.
> The code is based on the previous ibm_emac driver stuff. The 440EP/440GR
> allows controlling each EMAC clock spearately as opposed to global clock
> selection for 440GX.
> 
> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
> ---

[snip]

> +/* EMAC PHY clock workaround:
> + * 440EP/440GR has more sane SDR0_MFR register implementation than 440GX,
> + * which allows controlling each EMAC clock
> + */
> +static inline void emac_rx_clk_tx(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);
> +	}
> +}
> +
> +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