Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <9d439214e8c83ebf7b93dccca2f848fbaf75b9d4.camel@perches.com>
Subject: Re: [PATCH v2 1/7] media: uvcvideo: Use pr_cont() macro
From:   Joe Perches <joe@perches.com>
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     Ricardo Ribalda <ribalda@chromium.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Wed, 04 Nov 2020 13:51:29 -0800
In-Reply-To: <20201104214201.GH29958@pendragon.ideasonboard.com>
References: <20201104180734.286789-1-ribalda@chromium.org>
         <20201104180734.286789-2-ribalda@chromium.org>
         <87769d554b4575bf9371380b013e66d70f1b21c4.camel@perches.com>
         <20201104214201.GH29958@pendragon.ideasonboard.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2020-11-04 at 23:42 +0200, Laurent Pinchart wrote:
> Hi Joe,

Hi Laurent.

> On Wed, Nov 04, 2020 at 11:29:30AM -0800, Joe Perches wrote:
> > On Wed, 2020-11-04 at 19:07 +0100, Ricardo Ribalda wrote:
> > > Replace all the uses of printk(KERN_CONT ... with pr_cont().
> > 
> > Perhaps remove the uvc_printk macro and uses and use the more
> > common pr_fmt and pr_<level> mechanisms.
> 
> I'd actually go for dev_* instead, to give some context. It's fairly
> common to have multiple UVC devices connected to a system, so printing
> the device name would be useful. It can still be wrapped with
> uvc_printk() if we want to wrap the cast from uvc_device to a struct
> device (we should actually try to get the device corresponding to the
> USB interface where available, so we should use uvc_streaming->intf->dev
> where possible, and fallback to uvc_device->udev->dev otherwise), or
> drop the wrapper completely.

Of course yes.  I was not going to look around and update the existing
call sites to find whatever controlling uvc_device * or other struct *
to a real device that exists though.

It's not even clear from the changes that an appropriate pointer to
some struct exists in all the functions.

That's work for someone that knows the actual subsystem and I do not.

cheers, Joe