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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <Z5TGPS.PN2YJKFH2CDV@unrealasia.net>
Date: Thu, 02 Jan 2025 22:20:23 +0800
From: Muhammad Nuzaihan <zaihan@...ealasia.net>
To: Andrew Lunn <andrew@...n.ch>
Cc: netdev@...r.kernel.org, Loic Poulain <loic.poulain@...aro.org>,
	Sergey Ryazanov <ryazanov.s.a@...il.com>, Johannes Berg
	<johannes@...solutions.net>
Subject: Re: [PATCH] Add NMEA GPS character device for PCIe MHI Quectel Module
 to read NMEA statements.

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@...n.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