On 2023/4/7 17:57, Conor Dooley wrote: > Hey Minda, > > On Fri, Apr 07, 2023 at 10:32:51AM +0800, Minda Chen wrote: >> On 2023/4/6 19:54, Conor Dooley wrote: >> > On Thu, Apr 06, 2023 at 12:47:41PM +0100, Conor Dooley wrote: >> >> On Thu, Apr 06, 2023 at 07:11:39PM +0800, Minda Chen wrote: >> >> > This patchset adds PCIe driver for the StarFive JH7110 SoC. >> >> > The patch has been tested on the VisionFive 2 board. The test >> >> > devices include M.2 NVMe SSD and Realtek 8169 Ethernet adapter. >> >> >> >> I was talking with Daire last week about some changes he's working on >> >> for the microchip driver, and we seemed to recall an off-list email >> >> sent to Daire & Bjorn about extracting the common PLDA bits from the >> >> pcie-microchip-host driver to be used with an (at that point) >> >> unreleased SoC. Perhaps Bjorn has this in his mailbox somewhere still, >> >> our corporate mail policy scrubs things from over a year ago & I could >> >> not find it. >> >> >> >> I realised that that may actually have been StarFive, and the driver on >> >> your GitHub [1] certainly felt very familiar to Daire (he said it was >> >> very similar to his earlier revisions of his driver). >> >> >> >> I've not looked at a diff between this and the version you ship on >> >> GitHub, but first a quick inspection it mostly just looks like you >> >> did s/plda/sifive/ on the file. >> >> >> >> I'm obviously not a PCI maintainer, but if there are common bits between >> >> the two drivers, extracting common bits seems like a good idea to me... > >> Thanks. It is pleasure to using same common codes. Does common bits changes >> will upstream soon? > > I don't quite get what you mean. We've got some changes that are in > progress here: > https://lore.kernel.org/linux-pci/20230111125323.1911373-1-daire.mcnamara@xxxxxxxxxxxxx/ > We've been quiet there for a while, but Daire's back looking into Robin's > comments in there about the range parsing/window setup at the moment. > > I'm not sure if that's what you mean though, since you said "common > bits" & Daire was doing that work in a world where there was no jh7110 > driver in the mix. > Extracting common bits would be part of the process of adding a new > driver, as I don't think there's any real reason to do so without > another in-tree user. > OK, I know extracting common bits is microchip new PCIe driver codes changed. Just ignore my previous comments. Maybe I will try to restructuring the driver code according to corporate e-mail which has been sent one year ago. >> And I see there are many difference between pcie-microchip-host and our codes. > > Right. I'd expect there to be a fair difference between our integrations > of the IP, and therefore there'll be a bunch of non-shareable bits. > > You need the stg,syscon & phy bits, and the clock/reset handling is > clearly different too. > >> >> https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_devel/drivers/pci/controller/pcie-plda.c > > I had a bit of a read through this again today with Daire to check what > the differences actually are and it *looked* like the main, > non-implementation related, differences were the extra "event" domain > that was created to simplify the driver & the bottom half interrupt > handling. > That all came out of the review process, so it's likely that some of the > same requests would be made of you by the PCI maintainers anyway. > Thanks. I will check it and change my codes. > As an aside, you should probably run checkpatch --strict on this > submission, there's a rake of coding style "issues" in the new code > you've added. > I do not run checkpatch with "--strict". I will run with it. > Cheers, > Conor.
