2014-01-17 11:43:25

by Dorau, Lukasz

Subject: [PATCH] libahci: correct a misleading comment

There is an error in a comment introduced by:
commit fa070ee6dc70bcc19737a2406d741b089b3149d5
libahci: fix turning on LEDs in ahci_start_port()

Correct it.

Signed-off-by: Lukasz Dorau <[email protected]>
---
drivers/ata/libahci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index c482f8c..1f526e6 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -780,8 +780,8 @@ static void ahci_start_port(struct ata_port *ap)
4);
/*
* If busy, give a breather but do not
- * release EH ownership by using msleep()
- * instead of ata_msleep(). EM Transmit
+ * release EH ownership by using ata_msleep()
+ * instead of msleep(). EM Transmit
* bit is busy for the whole host and
* releasing ownership will cause other
* ports to fail the same way.

2014-01-17 12:00:23

by Tejun Heo

Subject: Re: [PATCH] libahci: correct a misleading comment

On Fri, Jan 17, 2014 at 12:43:20PM +0100, Lukasz Dorau wrote:
> There is an error in a comment introduced by:
> commit fa070ee6dc70bcc19737a2406d741b089b3149d5
> libahci: fix turning on LEDs in ahci_start_port()
>
> Correct it.
>
> Signed-off-by: Lukasz Dorau <[email protected]>
> ---
> drivers/ata/libahci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index c482f8c..1f526e6 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -780,8 +780,8 @@ static void ahci_start_port(struct ata_port *ap)
> 4);
> /*
> * If busy, give a breather but do not
> - * release EH ownership by using msleep()
> - * instead of ata_msleep(). EM Transmit
> + * release EH ownership by using ata_msleep()
> + * instead of msleep(). EM Transmit

Can't the sentence be parsed to mean the same thing either way? "(do
not release EH ownership) by using msleep..." or "do not (release EH
ownership by using ata_msleep)..."

With the context, I don't think which way to group things is
ambiguous. In fact, saying "instead of msleep()" is kinda confusing
because using ata_msleep() should be the default.

Thanks.

--
tejun

2014-01-17 12:10:51

by Dorau, Lukasz

Subject: RE: [PATCH] libahci: correct a misleading comment

On Friday, January 17, 2014 1:00 PM Tejun Heo <[email protected]> wrote:
> On Fri, Jan 17, 2014 at 12:43:20PM +0100, Lukasz Dorau wrote:
> > There is an error in a comment introduced by:
> > commit fa070ee6dc70bcc19737a2406d741b089b3149d5
> > libahci: fix turning on LEDs in ahci_start_port()
> >
> > Correct it.
> >
> > Signed-off-by: Lukasz Dorau <[email protected]>
> > ---
> > drivers/ata/libahci.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index c482f8c..1f526e6 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -780,8 +780,8 @@ static void ahci_start_port(struct ata_port *ap)
> > 4);
> > /*
> > * If busy, give a breather but do not
> > - * release EH ownership by using msleep()
> > - * instead of ata_msleep(). EM Transmit
> > + * release EH ownership by using ata_msleep()
> > + * instead of msleep(). EM Transmit
>
> Can't the sentence be parsed to mean the same thing either way? "(do
> not release EH ownership) by using msleep..." or "do not (release EH
> ownership by using ata_msleep)..."
>
> With the context, I don't think which way to group things is
> ambiguous. In fact, saying "instead of msleep()" is kinda confusing
> because using ata_msleep() should be the default.
>

You are right. I give up ;-)

Thanks,
Lukasz