All of lore.kernel.org
 help / color / mirror / Atom feed
From: Claudiu Manoil <claudiu.manoil@freescale.com>
To: David Laight <David.Laight@ACULAB.COM>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Xiubo Li <Li.Xiubo@freescale.com>,
	Shruti Kanetkar <Shruti@freescale.com>,
	"Kim Phillips" <kim.phillips@freescale.com>
Subject: Re: [net v2 3/8] net/fsl_pq_mdio: Replace spin_event_timeout() with arch independent
Date: Tue, 7 Oct 2014 15:16:10 +0300	[thread overview]
Message-ID: <5433D98A.2060904@freescale.com> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C5BAC@AcuExch.aculab.com>

On 10/7/2014 11:12 AM, David Laight wrote:
> From: netdev-owner@vger.kernel.
>> spin_event_timeout() is PPC dependent, use an arch independent
>> equivalent instead.
>
> I think you should white a local function/#define that expands to spin_event_timeout()
> on ppc and to the code below you are substituting on other architectures.
>
>>   	/* Wait for the transaction to finish */
>> -	status = spin_event_timeout(!(ioread32be(&regs->miimind) &
>> -				    MIIMIND_BUSY), MII_TIMEOUT, 0);
>> +	timeout = MII_TIMEOUT;
>> +	while ((ioread32be(&regs->miimind) & MIIMIND_BUSY) && timeout) {
>> +		cpu_relax();
>> +		timeout--;
>> +	}
>
> 	David
>
>
>

Hi David,

This point is debatable. Still better than adding a new local function 
would be to provide an implementation for spin_even_timeout() for ARM.
But it is not the place of the ethernet driver to provide an 
implementation of spin_event_timeout() on ARM.
Instead, I opted to simplify the busy wait/ timeout mechanism
in the driver using a simple and arch independent implementation 
replacing the PPC specific spin_event_timeout().  This timeout 
implementation, open-coded as a while() loop, is commonly used by other 
drivers too and I think that for this particular driver (fsl_pq_mdio) it 
is good enough to do the job, while keeping the code simple and 
portable.  It may not be as precise as spin_event_timeout() on PPC, but 
good enough.

Thanks,
Claudiu

  reply	other threads:[~2014-10-07 12:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-07  7:44 [net v2 0/8] gianfar: ARM port driver updates (1/2) Claudiu Manoil
2014-10-07  7:44 ` [net v2 1/8] net/fsl_pq_mdio: Fix asm/ucc.h compile error for ARM Claudiu Manoil
2014-10-07  7:44 ` [net v2 2/8] net/fsl_pq_mdio: Use ioread/iowrite32be() portable accessors Claudiu Manoil
2014-10-07  7:44 ` [net v2 3/8] net/fsl_pq_mdio: Replace spin_event_timeout() with arch independent Claudiu Manoil
2014-10-07  8:12   ` David Laight
2014-10-07 12:16     ` Claudiu Manoil [this message]
2014-10-07  7:44 ` [net v2 4/8] gianfar: Include missing headers for ARM builds Claudiu Manoil
2014-10-07  7:44 ` [net v2 5/8] gianfar: Exclude PPC specific errata handling from " Claudiu Manoil
2014-10-07  7:44 ` [net v2 6/8] gianfar: Make MAC addr setup endian safe, cleanup Claudiu Manoil
2014-10-07  7:44 ` [net v2 7/8] gianfar: Replace spin_event_timeout() with arch independent Claudiu Manoil
2014-10-07  7:44 ` [net v2 8/8] gianfar: Replace eieio with wmb for non-PPC archs Claudiu Manoil
2014-10-08  0:35 ` [net v2 0/8] gianfar: ARM port driver updates (1/2) David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5433D98A.2060904@freescale.com \
    --to=claudiu.manoil@freescale.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=Li.Xiubo@freescale.com \
    --cc=Shruti@freescale.com \
    --cc=davem@davemloft.net \
    --cc=kim.phillips@freescale.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.