Received-SPF: pass (google.com: domain of linux-kernel+bounces-39456-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240125004456.575891-1-abhishekpandit@google.com>
 <20240124164443.v2.2.I3d909e3c9a200621e3034686f068a3307945fd87@changeid> <2024012512-haphazard-mobster-f566@gregkh>
In-Reply-To: <2024012512-haphazard-mobster-f566@gregkh>
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Date: Thu, 25 Jan 2024 16:21:47 -0800
Message-ID: <CANFp7mWzA5df9iFpCWFRpXuOP06yDmBehYDYNACjrW2fPvp_Ow@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] usb: typec: ucsi: Update connector cap and status
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Abhishek Pandit-Subedi <abhishekpandit@google.com>, 
	Heikki Krogerus <heikki.krogerus@linux.intel.com>, linux-usb@vger.kernel.org, 
	jthies@google.com, pmalani@chromium.org, 
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>, Neil Armstrong <neil.armstrong@linaro.org>, 
	Rajaram Regupathy <rajaram.regupathy@intel.com>, Saranya Gopal <saranya.gopal@intel.com>, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Jan 25, 2024 at 3:03=E2=80=AFPM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 24, 2024 at 04:44:53PM -0800, Abhishek Pandit-Subedi wrote:
> > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucs=
i.h
> > index bec920fa6b8a..94b373378f63 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.h
> > +++ b/drivers/usb/typec/ucsi/ucsi.h
> > @@ -3,6 +3,7 @@
> >  #ifndef __DRIVER_USB_TYPEC_UCSI_H
> >  #define __DRIVER_USB_TYPEC_UCSI_H
> >
> > +#include <asm-generic/unaligned.h>
>
> Do you really need to include a asm/ include file?  This feels very
> wrong.

I didn't see any header in include/linux that already had these
unaligned access functions so I opted to include
asm-generic/unaligned.h. Is there a reason not to use an asm/ include
file?

>
> It's also in the wrong place, AND why "asm-generic"?  That also feels
> wrong.

asm-generic is definitely wrong; I misunderstood how these headers are
supposed to be used (should be just asm/unaligned.h).
For ordering, I took a look at some other files and it looks like
<asm/...> goes below the <linux/...> includes. This also probably
deserves documenting in the style guide.

>
> thanks,
>
> greg k-h