linux-kernel - Re: [RFC PATCH v1 12/12] usb: typec: qcom: define the bridge's path

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: <ZQRKq7K8jKlH/Y4X@kuha.fi.intel.com>
Date:   Fri, 15 Sep 2023 15:14:35 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Janne Grunau <j@...nau.net>, Simon Ser <contact@...rsion.fr>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        freedreno@...ts.freedesktop.org
Subject: Re: [RFC PATCH v1 12/12] usb: typec: qcom: define the bridge's path

Hi Dmitry,

On Mon, Sep 04, 2023 at 12:41:50AM +0300, Dmitry Baryshkov wrote:
> In order to notify the userspace about the DRM connector's USB-C port,
> export the corresponding port's name as the bridge's path field.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> ---
>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c     | 11 +++++++----
>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.c |  4 +++-
>  drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_drm.h |  6 ++++--
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> index b9d4856101c7..452dc6437861 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec.c
> @@ -156,6 +156,7 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
>  	struct device_node *np = dev->of_node;
>  	const struct pmic_typec_resources *res;
>  	struct regmap *regmap;
> +	char *tcpm_name;
>  	u32 base[2];
>  	int ret;
>  
> @@ -211,10 +212,6 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
>  	mutex_init(&tcpm->lock);
>  	platform_set_drvdata(pdev, tcpm);
>  
> -	tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev);
> -	if (IS_ERR(tcpm->pmic_typec_drm))
> -		return PTR_ERR(tcpm->pmic_typec_drm);
> -
>  	tcpm->tcpc.fwnode = device_get_named_child_node(tcpm->dev, "connector");
>  	if (!tcpm->tcpc.fwnode)
>  		return -EINVAL;
> @@ -225,6 +222,12 @@ static int qcom_pmic_typec_probe(struct platform_device *pdev)
>  		goto fwnode_remove;
>  	}
>  
> +	tcpm_name = tcpm_port_get_name(tcpm->tcpm_port);
> +	tcpm->pmic_typec_drm = qcom_pmic_typec_init_drm(dev, tcpm_name);

So I got some questions and concerns off-list. This was one of the
concerns. That tcpm_name is now the actual port device name, so I'm
afraid this is not acceptable.

You can't use device name as a reference, ever. There is no way to
guarantee that a device with a specific name is what you meant it to
be by the time it's accessed.

If you need to deal with a device, then you have to get an actual
reference to it (class_find_device_by_fwnode() should work in this
case).

Ideally you would get the reference in the place where you actually
use it (so drm_connector.c or more likely drm_sysfs.c) but that would
mean a dependency on typec in there, if the component framework or
something like that (device links?) is not an option. You could of
course try to confine the dependency somehow. drm_class does not have
implementation for dev_uevent, so you could take over that as a
temporary solution.

The only way to avoid the dependency completely would be to pass that
device reference from here through your drm bridge chain somehow.
But that's also really fragile. But it could be acceptable as a
temporary solution perhaps, if it's even possible.

Br,

-- 
heikki