On Mon, Nov 12 2018, Sergio Paracuellos wrote:

> On Mon, Nov 12, 2018 at 08:40:10AM +1100, NeilBrown wrote:
>> On Sun, Nov 11 2018, Greg KH wrote:
>> 
>> > On Sun, Nov 04, 2018 at 11:49:26AM +0100, Sergio Paracuellos wrote:
>> >> This patch series parse remaining port info from device tree storing
>> >> it in mt7621_pcie_port struct created for this. It also performs a lot
>> >> of cleanups to get the driver in a good shape to give it a try to get
>> >> mainlined. All of this changes are only compile-tested.
>> >
>> > Given the lack of responses here, I guess I'll just merge this and see
>> > what happens :)
>> 
>> Sounds like a good plan.
>> I had meant to look at it this past weekend, but ran out of time.
>> It is a bit awkward for me to test on mainline at the moment as
>> 
>> # first bad commit: [f8c55dc6e828324fc58c0bb32d72a5a4041d1c3b] MIPS: use generic dma noncoherent ops for simple noncoherent platforms
>> 
>> breaks mmc on my hardware, and my root filesystem is on mmc.
>> 
>> But I should still be able to get it tested sometime in the next couple
>> of weeks, and will provide feedback once I have it.
>
> Thanks, Neil. Please, let me know if I can help in any way.

I've got all the way to the end of the series and with the fixes that
I've already posted, my device still works.
There are lots of nice clean-ups in there - thanks!  I didn't review
them very closely as I was mostly focused on testing but what I saw
generally looked nice.

For the clock issue, I would just make a missing driver non-fatal.
clk_enable() is a no-op on ralink-mips, and I'm not sure that
clk_prepare does much either.

Handling the reset issue is a bit harder.
It seems that most bits in the reset register are
 1=assert  0=deassert
but that on some chips, the three PCI reset lines are inverted.
It would be easiest to put a quirk in  arch/mips/ralink/reset.c 
to check the CHIP_REV for the three lines and invert.

It might be cleaner to add some information to devicetree, but I cannot
easily find any precedent for that.

BTW, rather than calling
	reset_control_deassert(port->pcie_rst);
	reset_control_assert(port->pcie_rst);

maybe we should
	reset_control_reset(port->pcie_rst);
and not worry about a delay.

Are you OK to submit patches to address the various issues that I found?

Thanks a lot,
NeilBrown