Message-ID: <f1e5eb1c-ed1f-96d7-ecf5-69b467ed8339@iki.fi>
Date:   Tue, 14 Mar 2023 10:00:22 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v3] USB: serial: xr: Add TIOCGRS485 and TIOCSRS485 ioctls
Content-Language: en-US
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Johan Hovold <johan@kernel.org>, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230313010416.845252-1-kasper@iki.fi>
 <20230314070002.1008959-1-kasper@iki.fi> <ZBAkOr0epPPICLNP@kroah.com>
From:   Jarkko Sonninen <kasper@iki.fi>
In-Reply-To: <ZBAkOr0epPPICLNP@kroah.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 3/14/23 09:37, Greg Kroah-Hartman wrote:
> On Tue, Mar 14, 2023 at 09:00:01AM +0200, Jarkko Sonninen wrote:
>> Add support for RS-485 in Exar USB adapters.
>> RS-485 mode is controlled by TIOCGRS485 and TIOCSRS485 ioctls.
>> Gpio mode register is set to enable RS-485.
>>
>> Signed-off-by: Jarkko Sonninen <kasper@iki.fi>
>> ---
>>
>> In this version only rs485.flags are stored to state.
>> There is no locking as only one bit of the flags is used.
>> ioctl returns -ENOIOCTLCMD as the actual error handling is in tty code.
> And the difference between previous versions?  Take a look at the
> documentation for how to better describe version differences please.

I'll try to be more precise in the future.

>> +	memset(&rs485, 0, sizeof(rs485));
> But you just copied this from userspace, why zero it out again?  Is that
> to be expected (I really don't remember, sorry).


serial.h says

/*
  * Serial interface for controlling RS485 settings on chips with suitable
  * support. Set with TIOCSRS485 and get with TIOCGRS485 if supported by 
your
  * platform. The set function returns the new state, with any 
unsupported bits
  * reverted appropriately.
  */

This may mean only the flags, but I decided to zero the other 
unsupported fields as well.


> Anyway, just minor comments, I'll let others review it as well.
>
> thanks,
>
> greg k-h

     - Jarkko