| Message ID | 20250423-fix_serdev-v1-1-26ca3403fd33@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | serdev: Get serdev controller's name by dev_name() | expand |
On Wed, Apr 23, 2025 at 10:27:00PM +0800, Zijun Hu wrote: > From: Zijun Hu <quic_zijuhu@quicinc.com> > > serdev_controller_add() uses hardcoded serdev controller's name, and that > may be wrong once user changes the name after serdev_controller_alloc(). > > Fix by using dev_name() instead of hardcoded name. > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > --- > drivers/tty/serdev/core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > index eb2a2e58fe78fbbdb5839232936a994bda86d0b4..971651b8e18dcbb5b7983cdfa19e7d60d4cd292b 100644 > --- a/drivers/tty/serdev/core.c > +++ b/drivers/tty/serdev/core.c > @@ -783,8 +783,8 @@ int serdev_controller_add(struct serdev_controller *ctrl) > goto err_rpm_disable; > } > > - dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n", > - ctrl->nr, &ctrl->dev); > + dev_dbg(&ctrl->dev, "%s registered: dev:%p\n", > + dev_name(&ctrl->dev), &ctrl->dev); dev_dbg() already has the name in it, so why repeat it again? thanks, greg k-h
On 2025/4/23 22:35, Greg Kroah-Hartman wrote: >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c >> index eb2a2e58fe78fbbdb5839232936a994bda86d0b4..971651b8e18dcbb5b7983cdfa19e7d60d4cd292b 100644 >> --- a/drivers/tty/serdev/core.c >> +++ b/drivers/tty/serdev/core.c >> @@ -783,8 +783,8 @@ int serdev_controller_add(struct serdev_controller *ctrl) >> goto err_rpm_disable; >> } >> >> - dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n", >> - ctrl->nr, &ctrl->dev); >> + dev_dbg(&ctrl->dev, "%s registered: dev:%p\n", >> + dev_name(&ctrl->dev), &ctrl->dev); > dev_dbg() already has the name in it, so why repeat it again? i guess the author wants to print a sentence which is easy to read. for built in name of dev_dbg(), it always happens at fixed location and not where good sentence wants. actually. drivers/tty/serdev/* have other such usages, for example. dev_dbg(&serdev->dev, "device %s registered\n", dev_name(&serdev->dev)); dev_err(&serdev->dev, "Can't add %s, status %pe\n",dev_name(&serdev->dev), ERR_PTR(err));
On Wed, Apr 23, 2025 at 11:07:03PM +0800, Zijun Hu wrote: > On 2025/4/23 22:35, Greg Kroah-Hartman wrote: > >> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > >> index eb2a2e58fe78fbbdb5839232936a994bda86d0b4..971651b8e18dcbb5b7983cdfa19e7d60d4cd292b 100644 > >> --- a/drivers/tty/serdev/core.c > >> +++ b/drivers/tty/serdev/core.c > >> @@ -783,8 +783,8 @@ int serdev_controller_add(struct serdev_controller *ctrl) > >> goto err_rpm_disable; > >> } > >> > >> - dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n", > >> - ctrl->nr, &ctrl->dev); > >> + dev_dbg(&ctrl->dev, "%s registered: dev:%p\n", > >> + dev_name(&ctrl->dev), &ctrl->dev); > > dev_dbg() already has the name in it, so why repeat it again? > > i guess the author wants to print a sentence which is easy to read. > > for built in name of dev_dbg(), it always happens at fixed location > and not where good sentence wants. > > actually. drivers/tty/serdev/* have other such usages, for example. > > dev_dbg(&serdev->dev, "device %s registered\n", dev_name(&serdev->dev)); > dev_err(&serdev->dev, "Can't add %s, status > %pe\n",dev_name(&serdev->dev), ERR_PTR(err)); Then those should also be fixed.
On 2025/4/24 00:56, Greg Kroah-Hartman wrote: >> actually. drivers/tty/serdev/* have other such usages, for example. >> >> dev_dbg(&serdev->dev, "device %s registered\n", dev_name(&serdev->dev)); >> dev_err(&serdev->dev, "Can't add %s, status >> %pe\n",dev_name(&serdev->dev), ERR_PTR(err)); > Then those should also be fixed. okay. let me append a patch to fix them as well.
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c index eb2a2e58fe78fbbdb5839232936a994bda86d0b4..971651b8e18dcbb5b7983cdfa19e7d60d4cd292b 100644 --- a/drivers/tty/serdev/core.c +++ b/drivers/tty/serdev/core.c @@ -783,8 +783,8 @@ int serdev_controller_add(struct serdev_controller *ctrl) goto err_rpm_disable; } - dev_dbg(&ctrl->dev, "serdev%d registered: dev:%p\n", - ctrl->nr, &ctrl->dev); + dev_dbg(&ctrl->dev, "%s registered: dev:%p\n", + dev_name(&ctrl->dev), &ctrl->dev); return 0; err_rpm_disable: