Re: [PATCH v6 00/33] staging: mt7621-pci: Parse ports info from DT and other minor cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Nov 24, 2018 at 1:21 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> 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.

Thanks for your effort in reviewing and testing this series. There
were a bit long
and with lots of changes and cleanups.

>
> 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.

Do you mean just remove clocks from bindings and its clk_* references in
code?

>
> 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.

I'll see how to achieve this issue. Is it ok to submit the patch to staging
if I add a quirk in the mainlined reset.c file? Include linux-mips
mail list in the
CC is enough?

>
> 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.

I'll also check this.

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

I'll do my best to try to fix this up.

>
> Thanks a lot,
> NeilBrown

Best regards,
    Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux