netdev - Re: [PATCH v2 net] ptp: ocp: adjust serial port symlink creation

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: <20240523083943.6ecb60d9@kernel.org>
Date: Thu, 23 May 2024 08:39:43 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby
 <jirislaby@...nel.org>
Cc: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Jonathan Lemon
 <jonathan.lemon@...il.com>, linux-serial@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH v2 net] ptp: ocp: adjust serial port symlink creation

On Fri, 10 May 2024 11:04:05 +0000 Vadim Fedorenko wrote:
> The commit b286f4e87e32 ("serial: core: Move tty and serdev to be children
> of serial core port device") changed the hierarchy of serial port devices
> and device_find_child_by_name cannot find ttyS* devices because they are
> no longer directly attached. Add some logic to restore symlinks creation
> to the driver for OCP TimeCard.
> 
> Fixes: b286f4e87e32 ("serial: core: Move tty and serdev to be children of serial core port device")
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> ---
> v2:
>  add serial/8250 maintainers
> ---
>  drivers/ptp/ptp_ocp.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
> index ee2ced88ab34..50b7cb9db3be 100644
> --- a/drivers/ptp/ptp_ocp.c
> +++ b/drivers/ptp/ptp_ocp.c
> @@ -25,6 +25,8 @@
>  #include <linux/crc16.h>
>  #include <linux/dpll.h>
>  
> +#include "../tty/serial/8250/8250.h"

Hi Greg, Jiri, does this look reasonable to you?
The cross tree include raises an obvious red flag.

Should serial / u8250 provide a more official API?
Can we use device_for_each_child() to deal with the extra
layer in the hierarchy?

>  #define PCI_VENDOR_ID_FACEBOOK			0x1d9b
>  #define PCI_DEVICE_ID_FACEBOOK_TIMECARD		0x0400
>  
> @@ -4330,11 +4332,9 @@ ptp_ocp_symlink(struct ptp_ocp *bp, struct device *child, const char *link)
>  }
>  
>  static void
> -ptp_ocp_link_child(struct ptp_ocp *bp, const char *name, const char *link)
> +ptp_ocp_link_child(struct ptp_ocp *bp, struct device *dev, const char *name, const char *link)
>  {
> -	struct device *dev, *child;
> -
> -	dev = &bp->pdev->dev;
> +	struct device *child;
>  
>  	child = device_find_child_by_name(dev, name);
>  	if (!child) {
> @@ -4349,27 +4349,39 @@ ptp_ocp_link_child(struct ptp_ocp *bp, const char *name, const char *link)
>  static int
>  ptp_ocp_complete(struct ptp_ocp *bp)
>  {
> +	struct device *dev, *port_dev;
> +	struct uart_8250_port *port;
>  	struct pps_device *pps;
>  	char buf[32];
>  
> +	dev = &bp->pdev->dev;
> +
>  	if (bp->gnss_port.line != -1) {
> +		port = serial8250_get_port(bp->gnss_port.line);
> +		port_dev = (struct device *)port->port.port_dev;
>  		sprintf(buf, "ttyS%d", bp->gnss_port.line);
> -		ptp_ocp_link_child(bp, buf, "ttyGNSS");
> +		ptp_ocp_link_child(bp, port_dev, buf, "ttyGNSS");
>  	}
>  	if (bp->gnss2_port.line != -1) {
> +		port = serial8250_get_port(bp->gnss2_port.line);
> +		port_dev = (struct device *)port->port.port_dev;
>  		sprintf(buf, "ttyS%d", bp->gnss2_port.line);
> -		ptp_ocp_link_child(bp, buf, "ttyGNSS2");
> +		ptp_ocp_link_child(bp, port_dev, buf, "ttyGNSS2");
>  	}
>  	if (bp->mac_port.line != -1) {
> +		port = serial8250_get_port(bp->mac_port.line);
> +		port_dev = (struct device *)port->port.port_dev;
>  		sprintf(buf, "ttyS%d", bp->mac_port.line);
> -		ptp_ocp_link_child(bp, buf, "ttyMAC");
> +		ptp_ocp_link_child(bp, port_dev, buf, "ttyMAC");
>  	}
>  	if (bp->nmea_port.line != -1) {
> +		port = serial8250_get_port(bp->nmea_port.line);
> +		port_dev = (struct device *)port->port.port_dev;
>  		sprintf(buf, "ttyS%d", bp->nmea_port.line);
> -		ptp_ocp_link_child(bp, buf, "ttyNMEA");
> +		ptp_ocp_link_child(bp, port_dev, buf, "ttyNMEA");
>  	}
>  	sprintf(buf, "ptp%d", ptp_clock_index(bp->ptp));
> -	ptp_ocp_link_child(bp, buf, "ptp");
> +	ptp_ocp_link_child(bp, dev, buf, "ptp");
>  
>  	pps = pps_lookup_dev(bp->ptp);
>  	if (pps)