[U-Boot] [PATCH v2 4/7] usb: dwc2-otg: make regs_otg (in platdata) a uintptr_t

[Tom Rini]

On Thu, Jun 08, 2017 at 07:59:30AM -0600, Simon Glass wrote:
> Hi Marek,
> 
> On 8 June 2017 at 07:48, Marek Vasut <marex at denx.de> wrote:
> > On 06/08/2017 03:45 PM, Simon Glass wrote:
> >> Hi Marek,
> >>
> >> On 8 June 2017 at 06:33, Marek Vasut <marex at denx.de> wrote:
> >>> On 06/08/2017 05:34 AM, sjg at google.com wrote:
> >>>> On 06/07/2017 03:37 PM, Simon Glass wrote:
> >>>>> Hi Marek,
> >>>>>
> >>>>> On 7 June 2017 at 07:33, Marek Vasut <marex at denx.de> wrote:
> >>>>>> On 06/07/2017 03:28 PM, Simon Glass wrote:
> >>>>>>> Hi Marek,
> >>>>>>>
> >>>>>>> On 7 June 2017 at 06:55, Marek Vasut <marex at denx.de> wrote:
> >>>>>>>> On 06/07/2017 02:53 PM, Simon Glass wrote:
> >>>>>>>>> Hi Marek,
> >>>>>>>>>
> >>>>>>>>> On 7 June 2017 at 06:41, Marek Vasut <marex at denx.de> wrote:
> >>>>>>>>>> On 06/07/2017 02:38 PM, Simon Glass wrote:
> >>>>>>>>>>> +Tom for comment
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Marek,
> >>>>>>>>>>>
> >>>>>>>>>>> On 7 June 2017 at 00:27, Marek Vasut <marex at denx.de> wrote:
> >>>>>>>>>>>> On 06/07/2017 02:16 AM, Simon Glass wrote:
> >>>>>>>>>>>>> Hi,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On 6 June 2017 at 17:59, Dr. Philipp Tomsich
> >>>>>>>>>>>>> <philipp.tomsich at theobroma-systems.com> wrote:
> >>>>>>>>>>>>>> Simon,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 06 Jun 2017, at 23:09, Simon Glass <sjg at chromium.org> wrote:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Hi Philipp,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 6 June 2017 at 07:42, Philipp Tomsich
> >>>>>>>>>>>>>>> <philipp.tomsich at theobroma-systems.com> wrote:
> >>>>>>>>>>>>>>>> The regs_otg field in uintptr_t of the platform data structure for
> >>>>>>>>>>>>>>>> dwc2-otg has thus far been an unsigned int, but will eventually be
> >>>>>>>>>>>>>>>> casted into a void*.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> This raises the following error with GCC 6.3 and buildman:
> >>>>>>>>>>>>>>>>  ../drivers/usb/gadget/dwc2_udc_otg.c: In function 'dwc2_udc_probe':
> >>>>>>>>>>>>>>>>  ../drivers/usb/gadget/dwc2_udc_otg.c:821:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >>>>>>>>>>>>>>>>    reg = (struct dwc2_usbotg_reg *)pdata->regs_otg;
> >>>>>>>>>>>>>>>>          ^
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> This changes regs_otg to a uintptr_t to ensure that it is large enough
> >>>>>>>>>>>>>>>> to hold any valid pointer (and fix the associated warning).
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Changes in v2:
> >>>>>>>>>>>>>>>> - (new patch) fix a int-to-pointer cast warning for regs_otg in
> >>>>>>>>>>>>>>>>  dwc2-otg to fix a buildman failure for u-boot-rockchip/master at 2b19b2f
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> include/usb/dwc2_udc.h | 2 +-
> >>>>>>>>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>>>>>>>>>>
> >>>> Applied to u-boot-rockchip, thanks!
> >>>>
> >>>
> >>> This is clearly a USB patch ... why would it go through u-boot-rockchip?
> >>> But OK, yes, I see we have no structure in place and patches go through
> >>> whatever random tree these days.
> >>
> >> It is assigned to me in patchwork
> >
> > I see, USB patch assigned not to USB maintainer ... hmmmm ...
> 
> That is pretty typical if it is part of a series. It's just too hard
> to coordinate multiple maintainers picking up bits of a series.
> 
> patch 1 add board feature
> patch 2 add usb feature
> patch 3 enable usb on board
> 
> Patch 3 cannot be applied until both 1 and 2 are in mainline, meaning
> that we end up with this sequence:
> 
> patch 1 applied by board maintainer, send pull request
> patch 2 applied by USB, send pull request
> patch 3 applied by board maintainer
> 
> which is very slow and the feature cannot be tested until the end. I
> guess you know that already, but acking a patch is helpful as it
> allows them to stay together.

Generally speaking, and with the AB/RB of other relevant maintainers, I
endorse the idea that a logical series of reasonable size should not be
broken up into N smaller series to go in via N subtrees.  That said...

> >> and is needed to fix a build
> >> warning. It is tricky to deal with individual patches within a larger
> >> series since there are often dependencies. I had the same issue with
> >> video patches.
> >>
> >> Don't we normally try to keep series together?
> >
> > Don't we normally at least try to get AB/RB from the maintainer before
> > applying patches this way ?
> 
> Yes that is best. I took your 'applied to' as an ack ;-)

If someone wants to grab patches via their own subtree, as Marek does,
and in this case already had, it should go via that tree.  Do I fail to
get this right every time?  Yes.  Should I be better about this? Yes.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170608/5fce8230/attachment.sig>