Re: [LINUX PATCH v8 2/2] memory: pl353: Add driver for arm pl353 static memory controller

[Julia Cartwright]

On Mon, May 07, 2018 at 10:12:28AM +0000, Naga Sureshkumar Relli wrote:
> Hi Julia,
>
> Thanks for reviewing the patch and Sorry for my late reply. This patch
> went to junk folder, hence I didn't catch this patch.
>
> From: Julia Cartwright [mailto:julia@xxxxxx]
[..]
> > 
> > It would be easier to follow if you constructed your two patchsets with git format-patch --
> > thread.
> >
>
> I am using the same but with out --thread.
>
> > Or, merge them into a single patchset, especially considering the dependency between patches.
>
> But both are different patches, one for Device tree documentation and other for driver update.

Yes, I'm not proposing you merge _patches_ but _patchsets_.  You have
two patchsets, one for the SMC driver, and another for the NAND.  Given
that they depend on one another, it's helpful for reviewers if you sent
them all together, with a cover letter which describes the entire
patchset, changelog, it's dependencies, revision changelog, etc.

Something like:

   [PATCH v9 0/4] rawnand: memory: add support for PL353 static memory controller + NAND
      [PATCH v9 1/4] Devicetree: Add pl353 smc controller devicetree binding information
      [PATCH v9 2/4] memory: pl353: Add driver for arm pl353 static memory controller
      [PATCH v9 3/4] Documentation: nand: pl353: Add documentation for controller and driver
      [PATCH v9 4/4] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

Anyway, just with --thread enabled would be an improvement.

[..]
> > > --- a/drivers/memory/Kconfig
> > > +++ b/drivers/memory/Kconfig
> > > @@ -152,6 +152,13 @@ config DA8XX_DDRCTL
> > >  	  This driver is for the DDR2/mDDR Memory Controller present on
> > >  	  Texas Instruments da8xx SoCs. It's used to tweak various memory
> > >  	  controller configuration options.
> > > +config PL35X_SMC
> > > +	bool "ARM PL35X Static Memory Controller(SMC) driver"
> >
> > Is there any reason why this can't be tristate?
>
> There is a Nand driver which uses this driver. i.e The NAND driver
> Depends on this driver.

That's true, but it's irrelevant to question I asked.  It is perfectly
valid for both the SMC and NAND drivers to be tristate, why are you not
allowing this configuration?

   Julia