Hi,

"Steven J. Hill" <steven.h...@cavium.com> writes:
> On 09/08/2016 02:54 AM, Felipe Balbi wrote:
> [...]
>> 
>> octeon is going to be freed when ->remove() gets executed. You really
>> don't need to do these. In fact, setting usbctl to NULL will break
>> iounmap(). It seems to be me you don't need a remove at all.
>> 
> Hello Felipe.
>
> I trimmed the recipients down to just the USB list. Our original code
> had a start/setup hacked into the 'dwc3/core.c' file as shown below.
> This function was responsible for setting up the USB clocks so that
> the registers could be accessed. Obviously, this could not go upstream.
> I attempted to put all of the USB clock initialization into the probe()
> function in our 'dwc3-octeon.c' file. However, that approach does not
> work. The dwc3_probe() function is called before dwc3_octeon_probe()

this cannot happen, ever! dwc3-octeon is supposed to be a parent device
for dwc3. Look at how e.g. am4372.dtsi describes dwc3:

                dwc3_1: omap_dwc3@48380000 {
                        compatible = "ti,am437x-dwc3";
                        ti,hwmods = "usb_otg_ss0";
                        reg = <0x48380000 0x10000>;
                        interrupts = <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>;
                        #address-cells = <1>;
                        #size-cells = <1>;
                        utmi-mode = <1>;
                        ranges;

                        usb1: usb@48390000 {
                                compatible = "synopsys,dwc3";
                                reg = <0x48390000 0x10000>;
                                interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
                                             <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
                                             <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>;
                                interrupt-names = "peripheral",
                                                  "host",
                                                  "otg";
                                phys = <&usb2_phy1>;
                                phy-names = "usb2-phy";
                                maximum-speed = "high-speed";
                                dr_mode = "otg";
                                status = "disabled";
                                snps,dis_u3_susphy_quirk;
                                snps,dis_u2_susphy_quirk;
                        };
                };

> and the registers are not yet accessible. The dwc3 core code hangs
> when attempting to call dwc3_cache_hwparams() for this reason. So,
> instead I moved all the code out and into our platform code:
>
>    device_initcall(dwc3_octeon_device_init);

no, you shouldn't do that either.

> This works just fine, but it would have been nice to put all that
> code into the 'dwc3-octeon.c' probe() function. When looking at the

that's what you should do, yes. Then call of_platform_populate() after
everything is setup so your child dwc3 is created.

> other glue code files, they appear to be reading and writing dwc3
> registers just fine including clock registers on their platforms.
> Just want to make sure our approach is correct/sane. Thanks.

Seems like you're not describing your DTS properly. Can you show me how
you're describing dwc3-octeon and dwc3?

-- 
balbi

Attachment: signature.asc
Description: PGP signature

Reply via email to