RE: [Intel-wired-lan] [PATCH iwl-next, v1 2/3] igc: Add default Rx queue configuration via sysfs

[Song, Yoong Siang]

On Tuesday, July 30, 2024 9:20 PM, Marcin Szycik <marcin.szycik@xxxxxxxxxxxxxxx> wrote:
>On 30.07.2024 03:23, Song Yoong Siang wrote:
>> From: Blanco Alcaine Hector <hector.blanco.alcaine@xxxxxxxxx>
>>
>> This commit introduces the support to configure default Rx queue during
>
>Use imperative mood.
>

Hi Marcin Szycik,
Thanks for your review comments.
Sure, I will use imperative mood in the commit msg.
[...]

>> +/* MRQC register bit definitions */
>> +#define IGC_MRQC_ENABLE_MQ		0x00000000
>
>Just 0.
>

Noted.
[...]

>> +	if (queue > adapter->rss_queues - 1) {
>
>if (queue >= adapter->rss_queues)
>

Noted.
[...]

>> +static ssize_t default_rx_queue_show(struct device *dev,
>> +				     struct device_attribute *attr,
>> +				     char *buf)
>
>Why no igc_ prefix (and function doc)?
>

Sure. Will add igc prefix in the function name.
[...]

>> +static ssize_t default_rx_queue_store(struct device *dev,
>> +				      struct device_attribute *attr,
>> +				      const char *buf, size_t count)
>
>Ditto
>

Noted.
[...]

>> +	err = igc_set_default_rx_queue(adapter, default_rx_queue);
>> +	if (err < 0)
>> +		return -EINVAL;
>
>Why discard return error here?
>

Will use "return err" in v2 submission.
[...]

>
>Thanks,
>Marcin

Thanks & Regards
Siang