Re: [PATCH 00/11] Cleanup Octeon DWC3 glue code

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Hi,

On Fri, Jun 30, 2023, Ladislav Michl wrote:
> Hi,
> 
> On Mon, Jun 26, 2023 at 11:17:57PM +0000, Thinh Nguyen wrote:
> > Hi,
> [snip]
> > For each patch, please include all the emails returned from
> > get_maintainer.pl including Greg's and Thomas's.
> 
> I already explained why is Cc list constructed the way it is. Despite
> formal rules, time is a scarce resource, so let's not draw people's

Now that doesn't sound fair that you think my resource is more
expendable. :)

> attention too early. Anyway, if you think there's something that
> needs someone's special attention, you can still extend Cc list
> yourself.

I'm just asking you to follow the submitting patch process and add the
appropriate maintainers on any code that they maintain. You just ignored
the get_maintainer.pl output and only added what you feel needed.

> 
> Before sending v2, maintainer's review or ack is needed. I already
> collected Thomas' ack for a move, but have not read a single world
> from you. Do you plan to do some actual review, so we can take next
> step or are you willing to wait for a v2 which will add only

Please be patient, I'll get to your changes when I can, and I'm not the
only reviewer here. Your series isn't exactly small or easy to
understand for me to review quickly.

> octeon_get_io_clock_rate stub for non Octeon builds (having clk api
> would be nice here, but that's different story)?
> 
> Also, perhaps it would be reasonable to squash patches 8 and 9.
> Which tree to you want it to be rebased against? Currently I'm at
> linux.git master and changes were retested here.
> 
> I'd prefer to send fewer version if possible, so actual comments
> on code would certainly help.

Well, you can help me and other reviewers by submitting v2 with your
rebased changes. I think it's appropriate here given that this series
touches on 2 separate subsystems.

> 
> [snip]
> > > Also coleagues of mine meanwhile found that PLL indeed ocassionally
> > > fails to lock, so workaround attached to cover letter is really needed.
> > > Naturally it cannot sneak in as it is, so unless you have better idea
> > > I'll just port it to recent driver state and we can start discussion
> > > from there in a separate thread.
> > 
> > If this causes a regression, then please fix it before sending it in. If
> > it's a new found issue, you can create a fix patch at a later point.
> 
> There are few problems with Octeon's DWC3 implementation and none of them
> can be really solved without documentation. Here I come in trouble as
> Sysnopsys tech support pointed to SoC manufacturer which no longer exists.
> Cavium was bought by Marvell, dumped almost all the staff and those
> still providing technical support were unable to find any revelant
> documentation in past few months.

This can be problematic...

> 
> So unless that changes I can send only hackish patches which kinda
> overcome issues found, but I do not think it is viable solution. That's
> something I'll do at later point as you suggested and we'll see if an
> acceptable solution pops up.
> 

If it's hackish because of the lacking of documentation, then just note
how you came to the solution in your patch is totally fine. There's
nothing else we can do in that case.

I notice that there are many magic numbers in your changes. I hope you
have documentations for them. I may need to make some assumption/skip
reviewing some areas because of that.

Thanks,
Thinh