Re: [PATCH v10 5/6] spi: at91-usart: add driver for at91-usart as spi
On 13/07/2018 12:56:22+0300, Radu Pirea wrote: > > > On 07/10/2018 09:15 PM, Mark Brown wrote: > > On Mon, Jun 25, 2018 at 08:22:29PM +0300, Radu Pirea wrote: > > > > This is mostly good, just a couple of small things: > > > > > +config SPI_AT91_USART > > > + tristate "Atmel USART Controller SPI driver" > > > + depends on HAS_DMA > > > + depends on (ARCH_AT91 || COMPILE_TEST) > > > + select MFD_AT91_USART > > > > Why is this selecting rather than depending on the MFD like we normally > > do? > > > > Hi Mark, > > If I add depends instead of select, current defconfigs will be broken and > the proper driver will not be selected. Like this, there is no required > change in any defconfig. Selecting the serial or spi driver from menuconfig > makes the MFD completely transparent. I would say that this is true for the USART part but the SPI part can use a depends on as it didn't exist before. -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v10 5/6] spi: at91-usart: add driver for at91-usart as spi
On 07/10/2018 09:15 PM, Mark Brown wrote: On Mon, Jun 25, 2018 at 08:22:29PM +0300, Radu Pirea wrote: This is mostly good, just a couple of small things: +config SPI_AT91_USART + tristate "Atmel USART Controller SPI driver" + depends on HAS_DMA + depends on (ARCH_AT91 || COMPILE_TEST) + select MFD_AT91_USART Why is this selecting rather than depending on the MFD like we normally do? Hi Mark, If I add depends instead of select, current defconfigs will be broken and the proper driver will not be selected. Like this, there is no required change in any defconfig. Selecting the serial or spi driver from menuconfig makes the MFD completely transparent. @@ -0,0 +1,432 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for AT91 USART Controllers as SPI + * + * Copyright (C) 2018 Microchip Technology Inc. + * Author: Radu Pirea + */ Please use C++ comments for the whole block so it looks a bit more intentional. Ok. I will change it. Otherwise Reviwed-by: Mark Brown
Re: [PATCH v10 5/6] spi: at91-usart: add driver for at91-usart as spi
On Mon, Jun 25, 2018 at 08:22:29PM +0300, Radu Pirea wrote: This is mostly good, just a couple of small things: > +config SPI_AT91_USART > + tristate "Atmel USART Controller SPI driver" > + depends on HAS_DMA > + depends on (ARCH_AT91 || COMPILE_TEST) > + select MFD_AT91_USART Why is this selecting rather than depending on the MFD like we normally do? > @@ -0,0 +1,432 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for AT91 USART Controllers as SPI > + * > + * Copyright (C) 2018 Microchip Technology Inc. > + * Author: Radu Pirea > + */ Please use C++ comments for the whole block so it looks a bit more intentional. Otherwise Reviwed-by: Mark Brown signature.asc Description: PGP signature
