From: Jeff Garzik <jeff@garzik.org>
To: Tejun Heo <htejun@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>, linux-ide@vger.kernel.org
Subject: Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long
Date: Tue, 06 Nov 2007 12:53:14 -0500 [thread overview]
Message-ID: <4730AA0A.4090904@garzik.org> (raw)
In-Reply-To: <473048F7.2050004@gmail.com>
Tejun Heo wrote:
> Alan Cox wrote:
>> On Tue, 6 Nov 2007 14:39:03 +0900
>> Tejun Heo <htejun@gmail.com> wrote:
>>
>>> xfer_mask is unsigned int not unsigned long. Change ->mode_filter to
>>> take and return unsigned int.
>>>
>>> While at it, rename @adev of ata_pci_default_filter() to @dev for
>>> consistency.
>>>
>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> The filter type was purposefully unsigned long to allow for expansion (eg
>> for SWDMA) without breaking drivers. No problem with it changing but I'd
>> say "unsigned int" was the worst possible choice - its now shorter (no
>> room for expansion) and size dependant on arch. u32 would be shorter and
>> consistent across all platforms, ulong would have more room for expansion.
>
> I agree it should be one of u* types and am happy to convert. This
> patch is primarily for consistency as in libata-core, xfer_mask is
> unsigned int. My first proposal was u32 too but Jeff vetoed it. IIRC,
> Jeff has affection for machine-independent integral types. :-)
>
> Anyways, I think ulong is worse than uint because ulong differs in size
> between 32 and 64 archs. What's the good of extra 32bits in flags space
> if it's not there on 32bit archs? Also, we currently only use 20bits of
> xfermask, 12 extra bits seem enough for the time, especially as
> everything is moving over to SATA where xfermode doesn't really matter, no?
>
> Jeff, are you okay with u32 or 64?
unsigned long is IMO the best choice for bitmaps.
* it is a machine int on all(?) architectures
* it is 32-bit on 32-bit architectures
* it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces
* conversion to test_bit() and lib/bitmap.c interfaces as a future step
is trivial
* your structs inflate on 64-bit due to pointers anyway, so I see little
real consequence to using the lower 32 bits of an unsigned long as a
portable meme.
Jeff
next prev parent reply other threads:[~2007-11-06 17:53 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-06 5:38 [PATCHSET] libata: update timing and fix pata_amd transfer mode selection Tejun Heo
2007-11-06 5:38 ` [PATCH 01/12] ata_generic: unindent loop in generic_set_mode() Tejun Heo
2007-11-06 5:39 ` [PATCH 02/12] libata: export xfermode / PATA timing related functions Tejun Heo
2007-11-06 5:39 ` [PATCH 03/12] libata: clean up xfermode / PATA timing related stuff Tejun Heo
2007-11-06 5:39 ` [PATCH 04/12] libata: kill ata_id_to_dma_mode() Tejun Heo
2007-11-06 10:49 ` Alan Cox
2007-11-06 11:21 ` Tejun Heo
2007-11-24 1:13 ` Jeff Garzik
2007-11-06 5:39 ` [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long Tejun Heo
2007-11-06 10:51 ` Alan Cox
2007-11-06 10:59 ` Tejun Heo
2007-11-06 17:53 ` Jeff Garzik [this message]
2007-11-07 1:12 ` Tejun Heo
2007-11-24 1:13 ` Jeff Garzik
2007-11-24 1:13 ` Jeff Garzik
2007-11-27 8:42 ` Tejun Heo
2007-11-06 5:39 ` [PATCH 06/12] libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes() Tejun Heo
2007-11-06 10:54 ` Alan Cox
2007-11-06 11:00 ` Tejun Heo
2007-11-24 1:14 ` Jeff Garzik
2007-11-06 5:39 ` [PATCH 07/12] libata: fix ata_acpi_gtm_xfermask() Tejun Heo
2007-11-06 10:55 ` Alan Cox
2007-11-24 1:16 ` Jeff Garzik
2007-11-27 8:40 ` Tejun Heo
2007-11-06 5:39 ` [PATCH 08/12] libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi Tejun Heo
2007-11-06 10:59 ` Alan Cox
2007-11-06 11:09 ` Tejun Heo
2007-11-24 1:17 ` Jeff Garzik
2007-11-06 5:39 ` [PATCH 09/12] libata: implement ata_acpi_init_gtm() Tejun Heo
2007-11-06 5:39 ` [PATCH 10/12] libata: reimplement ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask() Tejun Heo
2007-11-06 5:39 ` [PATCH 11/12] libata: add ATA_CBL_PATA_IGN Tejun Heo
2007-11-06 10:59 ` Alan Cox
2007-11-06 11:02 ` Tejun Heo
2007-11-06 11:25 ` Alan Cox
2007-11-06 5:39 ` [PATCH 12/12] pata_amd: update mode selection for NV PATAs Tejun Heo
2007-11-06 10:59 ` Alan Cox
2007-11-23 1:08 ` [PATCHSET] libata: update timing and fix pata_amd transfer mode selection Tejun Heo
2007-11-24 1:18 ` Jeff Garzik
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=4730AA0A.4090904@garzik.org \
--to=jeff@garzik.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=htejun@gmail.com \
--cc=linux-ide@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.