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@xxxxxxxxx>
> ---
> 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)
