2012.12.19. 0:58 keltezéssel, Julian Calaby írta:
> Hi Gabor,
>
> One minor question:
>
> On Wed, Dec 19, 2012 at 3:22 AM, Gabor Juhos <juhosg@xxxxxxxxxxx> wrote:
>> An ioremap call is allowed to fail, however
>> the return value of that is not checked in
>> the 'rt2800pci_read_eeprom_soc' function.
>>
>> The patch adds the missing check, and makes
>> the function return an int value. The patch
>> also converts the 'rt2800_read_eeprom' and
>> 'rt2800_ops.read_eeprom' functions to return
>> an int value, so the error value can be
>> propagated up to the 'rt2800_validate_eeprom'
>> function.
>>
>> Signed-off-by: Gabor Juhos <juhosg@xxxxxxxxxxx>
>> ---
>> drivers/net/wireless/rt2x00/rt2800lib.c | 5 ++++-
>> drivers/net/wireless/rt2x00/rt2800lib.h | 6 +++---
>> drivers/net/wireless/rt2x00/rt2800pci.c | 17 ++++++++++++-----
>> drivers/net/wireless/rt2x00/rt2800usb.c | 4 +++-
>> 4 files changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
>> index 9224d87..5fc16dd 100644
>> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
>> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
>> @@ -970,14 +974,17 @@ static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
>> /*
>> * Device probe functions.
>> */
>> -static void rt2800pci_read_eeprom(struct rt2x00_dev *rt2x00dev)
>> +static int rt2800pci_read_eeprom(struct rt2x00_dev *rt2x00dev)
>> {
>> if (rt2x00_is_soc(rt2x00dev))
>> - rt2800pci_read_eeprom_soc(rt2x00dev);
>> - else if (rt2800pci_efuse_detect(rt2x00dev))
>> + return rt2800pci_read_eeprom_soc(rt2x00dev);
>> +
>> + if (rt2800pci_efuse_detect(rt2x00dev))
>> rt2800pci_read_eeprom_efuse(rt2x00dev);
>> else
>> rt2800pci_read_eeprom_pci(rt2x00dev);
>
> Would it make any sense to have rt2800pci_read_eeprom_efuse() and
> rt2800pci_read_eeprom_pci() return a value, simply for consistency
> with rt2800pci_read_eeprom_soc()?
I wanted to keep the change as minimal as possible, but this would make sense.
>
>> +
>> + return 0;
>> }
>>
>> static const struct ieee80211_ops rt2800pci_mac80211_ops = {
>> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
>> index 5c149b5..48de5c9 100644
>> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
>> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
>> @@ -735,13 +735,15 @@ static void rt2800usb_fill_rxdone(struct queue_entry *entry,
>> /*
>> * Device probe functions.
>> */
>> -static void rt2800usb_read_eeprom(struct rt2x00_dev *rt2x00dev)
>> +static int rt2800usb_read_eeprom(struct rt2x00_dev *rt2x00dev)
>> {
>> if (rt2800_efuse_detect(rt2x00dev))
>> rt2800_read_eeprom_efuse(rt2x00dev);
>> else
>> rt2x00usb_eeprom_read(rt2x00dev, rt2x00dev->eeprom,
>> EEPROM_SIZE);
>
> Same here.
Yes, especially that the 'rt2x00usb_eeprom_read' function returns an int value
already.
Thank you for the review!
-Gabor
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
