linux-kernel - Re: [PATCH] serial: samsung: add support for manual RTS setting

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: <5238A8B1.9080105@inov.pt>
Date:	Tue, 17 Sep 2013 20:08:33 +0100
From:	José Miguel Gonçalves <jose.goncalves@...v.pt>
To:	Tomasz Figa <t.figa@...sung.com>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org,
	Heiko Stübner <heiko@...ech.de>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>
Subject: Re: [PATCH] serial: samsung: add support for manual RTS setting

On 17-09-2013 16:21, Tomasz Figa wrote:
> I had the following scenario in mind:
> 1) enable CRTSCTS,
> 2) set RTS status using the IOCTL,
> 3) disable CRTSCTS,
> 4) do something,
> 5) enable CRTSCTS again.
>
> I would expect that the value set in point 2 would be still valid after
> point 5, while it will be reset.

Maybe I'm missing something but, as I see it, as soon as you enable AFC in the 
UART controller the RTS pin is no more manually controllable, so after point 5 the 
pin is set automattically by the controller according with the Rx FIFO contents. 
Don't you want to say instead that the value set in 2) will be valid in 4)?

>
>> Regarding port capability, if it's decided to validate it in
>> s3c24xx_serial_get_mctrl() and s3c24xx_serial_set_mctrl() it should also
>> be validated here. The question is how to validate for the full spectrum
>> of SoCs that this driver supports?
> Hmm, since the driver is already broken in this aspect, ignoring this in
> your patch might be fine for now. A follow up patch fixing this would be
> welcome, though. However I don't have any good idea how to implement this
> at the moment.
>
> First thing that comes to my mind is using the variant data structures to
> store information about per port capability (different port FIFO sizes are
> already handled like this), but this would imply splitting some of the
> groups, as S5PC100 supports modem control for different subset of ports
> than S3C64xx, while they both use the same variant data.
>
> Using device tree, this could be passed as an extra property, but some of
> the platforms using samsung serial driver can be booted without device
> tree, so it wouldn't cover all the cases.

So, as I understand, for now is enough to resubmit the patch with the UMCON 
register setting in s3c24xx_serial_set_termios() preserving the previous manual 
setting of the RTS pin, correct? I was thinking in something like this:

     umcon = rd_regb(port, S3C2410_UMCON);
     if (termios->c_cflag & CRTSCTS) {
         umcon |= S3C2410_UMCOM_AFC;
         umcon &= ~S3C2412_UMCON_AFC_8; // Disable RTS when RX FIFO contains 63 bytes
     } else {
         umcon &= ~S3C2410_UMCOM_AFC;
     }
     wr_regl(port, S3C2410_UMCON, umcon);

> Best regards,
> Tomasz
>
> P.S. Please remember to add all the relevant people to Cc when sending
> patches. You can use scripts/get_maintainer.pl to find them.

OK, sorry for that.

Bet regards,
José Gonçalves
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/