From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Wed, 25 Apr 2012 16:12:32 -0700 Subject: [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings In-Reply-To: <4F70A05C.7020004@boundarydevices.com> References: <1332716409-29225-1-git-send-email-eric.nelson@boundarydevices.com> <1332716409-29225-3-git-send-email-eric.nelson@boundarydevices.com> <4F702A6A.3000406@denx.de> <4F7070B6.6090200@boundarydevices.com> <4F70725F.8080105@denx.de> <4F70A05C.7020004@boundarydevices.com> Message-ID: <4F9884E0.7090403@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 03/26/2012 09:59 AM, Eric Nelson wrote: > On 03/26/2012 06:42 AM, Stefano Babic wrote: >> On 26/03/2012 15:35, Eric Nelson wrote: >>> On 03/26/2012 01:35 AM, Stefano Babic wrote: >>>> On 26/03/2012 01:00, Eric Nelson wrote: >>>>> V2 has been stripped of the board-independent changes and >>>>> > >>>> > >>>> >>>>> +#ifdef CONFIG_CMD_SATA >>>>> + >>>>> +int setup_sata(void) >>>>> +{ >>>>> + int rval = enable_sata_clock(); >>>> >>>> What about to return at this point if there is an error ? >>>> >>> I'm not sure I understand. Do you mean re-structure the code with >>> two returns like this? >> >> No, much easier - I find the code is easy to understand if it looks like >> if the function returns immediately in case of error. >> >> if (do_something()) >> return ERROR; >> >> < code when no error happens> >> >> Your enable_sata_clock() return only -1 in case of error. You could >> easy write: >> >> if (enable_sata_clock()) >> return -1 (or better a value in errno.h) > > > The choice of error code is better made inside enable_sata_clock(), > although I'm not really sure what error code to choose. > > The error occurs if the PLL fails to lock and would indicate that > something's horribly wrong. > > I'm guessing -EIO is probably the right choice. > > If you agree, I'll send V2 of "i.MX6: add enable_sata_clock()" and > V3 of "i.MX6: mx6q_sabrelite: add SATA bindings". > Hi Stefano, It looks like I was waiting for you and never did send V3 of this patch. I'll forward one shortly. Regards, Eric