From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cyrille Pitchen Subject: Re: [PATCH linux-next v4 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller Date: Tue, 25 Aug 2015 12:21:06 +0200 Message-ID: <55DC4192.6060906@atmel.com> References: <201508241303.52066.marex@denx.de> <55DB4EA6.9090807@atmel.com> <201508241945.33577.marex@denx.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Jonas Gorski , Marek Vasut Cc: Ben Hutchings , Mark Rutland , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Pawel Moll , Ian Campbell , =?UTF-8?B?UmFmYcWCIE1pxYJl?= =?UTF-8?B?Y2tp?= , nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Rob Herring , linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Gabor Juhos , Mark Brown , MTD Maling List , Kumar Gala , Brian Norris , David Woodhouse , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , "Bean Huo (beanhuo)" List-Id: devicetree@vger.kernel.org Le 25/08/2015 11:46, Jonas Gorski a =C3=A9crit : > On Mon, Aug 24, 2015 at 7:45 PM, Marek Vasut wrote: >> On Monday, August 24, 2015 at 07:04:38 PM, Cyrille Pitchen wrote: >>> Hi Marek, >> >> Hi! >> >>> Le 24/08/2015 13:03, Marek Vasut a =C3=A9crit : >>>> On Monday, August 24, 2015 at 12:14:00 PM, Cyrille Pitchen wrote: >>>>> This driver add support to the new Atmel QSPI controller embedded= into >>>>> sama5d2x SoCs. It expects a NOR memory to be connected to the QSP= I >>>>> controller. >> >> [...] >> >>>>> + /* Compute address parameters */ >>>>> + switch (cmd->enable.bits.address) { >>>>> + case 4: >>>>> + ifr |=3D QSPI_IFR_ADDRL; >>>>> + /*break;*/ /* fallback to the 24bit address case */ >>>> >>>> What's this commented out bit of code for ? :-) >>> >>> I just wanted to stress out there was no missing "break;". >>> I've reworded the comment to: >>> /* No "break" on purpose: fallback to the 24bit address case. */ >> >> Oh, the address is in bytes . I see, yes, it makes sense to be more >> explicit here about the purpose of the fallback. I think this change >> in the comment will make it easier for everyone who comes back in a >> few years and reads this code. >=20 > I think you are looking for the term "(switch case) fallthrough", not > "fallback". "Fallback" makes it sound like there is something missing= , > or an invalid state. >=20 >=20 > Jonas >=20 will be modified in the next series, thanks for the review! -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html