Re: [PATCH] Add NMEA GPS character device for PCIe MHI Quectel Module to read NMEA statements.

[Muhammad Nuzaihan <zaihan@unrealasia.net>]

Hi Andrew,

I'm actually new to netdev and the main kernel.org documentation on 
submitting patches 
(https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html) 
was not clear on the commit message (or i've missed it).

Thank you for pointing me to the netdev maintainer process page.

Anyway, i will make a new v3 patch tomorrow on a new thread to address 
the issues you mentioned. I'll follow the guidelines as you required.

Sorry for the mess.

Thank you,
Muhammad Nuzaihan

On Thu, Jan 2 2025 at 02:22:15 PM +0100, Andrew Lunn <andrew@lunn.ch> 
wrote:
> On Thu, Jan 02, 2025 at 05:12:41AM +0800, Muhammad Nuzaihan wrote:
>>  Hi netdev,
>> 
>>  I made a mistake in choosing AT mode IPC, which is incorrect. For 
>> NMEA
>>  streams it should use LOOPBACK for IPC. If it uses AT, i noticed 
>> that using
>>  gpsd will cause intermittent IOCTL errors which is caused when gpsd 
>> wants to
>>  write to the device.
>> 
>>  Attached is the patch.
> 
> This is not my area, so i cannot do a full review, but a few things to
> note.
> 
> Please start a new thread for each version of a patch, and wait at
> lest 24 hours between each version.
> 
> The commit message should be formal, since it will be part of the
> kernel history.
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html
> 
>>  @@ -876,7 +880,8 @@ static long wwan_port_fops_ioctl(struct file 
>> *filp, unsigned int cmd,
>>   	struct wwan_port *port = filp->private_data;
>>   	int res;
>> 
>>  -	if (port->type == WWAN_PORT_AT) {	/* AT port specific IOCTLs */
>>  +	if (port->type == WWAN_PORT_AT ||
>>  +			WWAN_PORT_NMEA) {	/* AT or NMEA port specific IOCTLs */
> 
> This looks wrong. || WWAN_PORT_NMEA will always be true, assuming
> WWAN_PORT_NMEA is not 0.
> 
>     Andrew
> 
> ---
> pw-bot: cr