From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 In-Reply-To: <1456155.YPH2fYoGgF@wuerfel> References: <1418695828-605-1-git-send-email-Sneeker.Yeh@tw.fujitsu.com> <20141222155936.GB12815@saruman> <1456155.YPH2fYoGgF@wuerfel> Date: Sun, 4 Jan 2015 22:32:46 +0800 Message-ID: Subject: Re: [PATCH 2/3] usb: dwc3: add Fujitsu Specific Glue layer From: Sneeker Yeh Content-Type: multipart/alternative; boundary=001a11c117c8941088050bd47229 To: Arnd Bergmann Cc: Felipe Balbi , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Greg Kroah-Hartman , Mathias Nyman , Grant Likely , Alan Stern , Paul Bolle , Hans de Goede , Thomas Pugliese , David Mosberger , Peter Griffin , Sylwester Nawrocki , Andrew Bresticker , Gregory CLEMENT , Yoshihiro Shimoda , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, Andy Green , Jassi Brar , Sneeker Yeh List-ID: --001a11c117c8941088050bd47229 Content-Type: text/plain; charset=UTF-8 HI Arnd, Thanks for reviewing this, 2014-12-30 18:12 GMT+08:00 Arnd Bergmann : > On Monday 29 December 2014 01:52:04 Sneeker Yeh wrote: > > > > +static int dwc3_mb86s70_remove_child(struct device *dev, void > *unused) > > > > +{ > > > > + struct platform_device *pdev = to_platform_device(dev); > > > > + > > > > + of_device_unregister(pdev); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static u64 dwc3_mb86s70_dma_mask = DMA_BIT_MASK(32); > > > > > > why ? Use dma_coerce_mask_and_coherent(). > > > > > > > okay. > > Actually that is still wrong: we use dma_coerce_mask_and_coherent() to > annotate drivers that have traditionally been forcing their own dma mask > by some other means and that need to be changed to something proper (after > finding out why they did it in the first place). > thanks,I remove this and test can be passed, of_platform_populate() makes dwc3 owns a default one and don't have to reference parent device's mask i set here. if later i worry dwc3 might still not get a proper mask from of_platform_populate() in our platform, then i shall set a proper one via platform bus notifier somewhere else, IIUC. > Since this is about a child device, the correct interface is to use > platform_device_register_full(). > > dwc3 core usually can be describe as sub node in platform glue device node: usb3host0: mb86s70_usb3host0 { compatible = "fujitsu,mb86s70-dwc3"; clocks = <&clock 0 1 1>; clock-names = "h_clk"; #address-cells = <2>; #size-cells = <1>; ranges; dwc3@32200000 { compatible = "synopsys,dwc3"; reg = <0 0x32200000 0x100000>; interrupts = <0 292 0x4>, <0 294 0x4>; }; }; so that make me able to use of_platform_populate() to create dwc3 device in our platform glue device. > Note that this will still only work on machines that have no dma offset, > iommu, swiotlb, special coherency requirements, or ones that need a > platform specific dma_map_ops. To make any of those work, the driver > would need to be changed to pass the correct device pointer into any > dma-mapping interfaces, so the settings can get set by the platform > code. We will need to do that for arm64 I assume. > thanks for the teaching, for me it's gold and i'm always enjoying absorbing these. Much appreciate, Sneeker > > Arnd > --001a11c117c8941088050bd47229 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
HI Arnd,

Thanks for reviewing this,
<= div>

201= 4-12-30 18:12 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
On Monday 29 D= ecember 2014 01:52:04 Sneeker Yeh wrote:
> > > +static int dwc3_mb86s70_remove_child(struct device *dev, vo= id *unused)
> > > +{
> > > +=C2=A0 =C2=A0 =C2=A0struct platform_device *pdev =3D to_pla= tform_device(dev);
> > > +
> > > +=C2=A0 =C2=A0 =C2=A0of_device_unregister(pdev);
> > > +
> > > +=C2=A0 =C2=A0 =C2=A0return 0;
> > > +}
> > > +
> > > +static u64 dwc3_mb86s70_dma_mask =3D DMA_BIT_MASK(32);
> >
> > why ? Use dma_coerce_mask_and_coherent().
> >
>
> okay.

Actually that is still wrong: we use dma_coerce_mask_and_coherent() = to
annotate drivers that have traditionally been forcing their own dma mask by some other means and that need to be changed to something proper (after<= br> finding out why they did it in the first place).

<= /div>
thanks,I remove this and test can be passed,
of_pl= atform_populate() makes dwc3 owns a default one and don't have to refer= ence parent device's mask i set here.

if later i worr= y dwc3 might still not get a proper mask from of_platform_populate() in our= platform,
then i shall set a proper one via platform bus no= tifier somewhere else, IIUC.


Since this is about a child device, the correct interface is to use
platform_device_register_full().


dwc3 core usually can be describe as sub node in = platform glue device node:

usb3host0: mb86s70_usb3host0 {
=C2=A0= =C2=A0=C2=A0 compatible =3D "fujitsu,mb86s70-dwc3";
=C2=A0=C2= =A0=C2=A0 clocks =3D <&clock 0 1 1>;
=C2=A0=C2=A0=C2=A0 clock-= names =3D "h_clk";
=C2=A0=C2=A0=C2=A0 #address-cells =3D <2= >;
=C2=A0=C2=A0=C2=A0 #size-cells =3D <1>;
=C2=A0=C2=A0=C2= =A0 ranges;

=C2=A0=C2=A0=C2=A0 dwc3@32200000 {
=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 compatible =3D "synopsys,dwc3";
=C2=A0=C2= =A0=C2=A0 =C2=A0=C2=A0=C2=A0 reg =3D <0 0x32200000 0x100000>;
=C2= =A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 interrupts =3D <0 292 0x4>,
=C2= =A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 <0 294 0x4>;
=C2=A0=C2=A0=C2=A0= };
};

so that make me able to use of_platform_populat= e() to create dwc3 device in our platform glue device.
=C2=A0
Note that this will still only work on machines that have no dma offset, iommu, swiotlb, special coherency requirements, or ones that need a
platform specific dma_map_ops. To make any of those work, the driver
would need to be changed to pass the correct device pointer into any
dma-mapping interfaces, so the settings can get set by the platform
code. We will need to do that for arm64 I assume.

=
thanks for the teaching, for me it's gold and i'm always= enjoying absorbing these.

Much appreciate,
Sneeker

=C2=A0

=C2=A0 =C2=A0 =C2=A0 =C2=A0 Arnd

--001a11c117c8941088050bd47229--