mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Nicholas Krause <xerofoify@gmail.com>
Cc: linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv2] drivers:ata: Remove unneeded fix me comment for the function,mv_print_info in sata_mv.c
Date: Fri, 26 Dec 2014 17:12:27 -0500	[thread overview]
Message-ID: <20141226221227.GC10889@htj.dyndns.org> (raw)
In-Reply-To: <1419630959-2889-1-git-send-email-xerofoify@gmail.com>

Hello, Nicholas.

On Fri, Dec 26, 2014 at 04:55:59PM -0500, Nicholas Krause wrote:
> Removes a unneed fix me comment for the function,mv_print_info in sata_mv.c.
> This function is correctly completed due to printing out needed info about 
> hardware to the kernel log buffer. The needed information that is being 
> printed successfully now to the callers of this function is the host 
> number of ports, the generation of hardware, the maximum q depth the
> hardware supports, if the hardware is either RAID or SCSI enabled and
> finally the IRQ mode enabled when called and the flag enabled with the
> respectful IRQ called. 

It's often more effective to discuss to conclusion on thread before
sending another patch.

When tracking down FIXME comments, it's often a good idea to look at
how the comment came to be by digging through the history.  git blame
on the file shows that the comment was added by 05b308e1df6d ("[PATCH]
libata: Marvell function headers") a long time ago.  Compared to then,
the function grew generation info printing, which could have been what
the original FIXME author had in mind or maybe not.  Regardless, given
that nobody has complained about lacking information over the years,
it can be argued that the FIXME is spurious.

So, I don't object to the patch itself.  These are just taking way too
much effort given that the actual gain is almost nil.  If you want to
continue to do this, please research about the history of FIXME and
construct and present a valid rationale for the change.  For this one,
listing what it prints out doesn't constitute rationale in itself.

Please do keep in mind these changes are ultimately frivolous.  If
they incur more cost than they're beneficial, they're pointless, so
please send properly formed and reasoned patches.  This probably is my
last response on the subject.

Thanks.

-- 
tejun

           reply	other threads:[~2014-12-26 22:12 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <1419630959-2889-1-git-send-email-xerofoify@gmail.com>]

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=20141226221227.GC10889@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xerofoify@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®