Re: [PATCH 2/9] crypto: qce: Add register defines
On Apr 3, 2014, at 11:17 AM, Stanimir Varbanov wrote: > Here are all register addresses and bit/masks used by the driver. > > Signed-off-by: Stanimir Varbanov > --- > drivers/crypto/qce/regs-v5.h | 327 +++ > 1 file changed, 327 insertions(+) > create mode 100644 drivers/crypto/qce/regs-v5.h This should be part of the first patch in the series, you shouldn’t have any patch that doesn’t build. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] crypto: qce: Add register defines
Hi, On 04/03/2014 07:24 PM, Kumar Gala wrote: > > On Apr 3, 2014, at 11:17 AM, Stanimir Varbanov wrote: > >> Here are all register addresses and bit/masks used by the driver. >> >> Signed-off-by: Stanimir Varbanov >> --- >> drivers/crypto/qce/regs-v5.h | 327 >> +++ >> 1 file changed, 327 insertions(+) >> create mode 100644 drivers/crypto/qce/regs-v5.h > > This should be part of the first patch in the series, you shouldn’t have any > patch that doesn’t build. Correct, I thought I mentioned in the cover letter that the series is splitted by files to easier the review. I planned to squash all in one patch after review is done. regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] crypto: qce: Add register defines
On Apr 3, 2014, at 11:33 AM, Stanimir Varbanov wrote: > Hi, > > On 04/03/2014 07:24 PM, Kumar Gala wrote: >> >> On Apr 3, 2014, at 11:17 AM, Stanimir Varbanov wrote: >> >>> Here are all register addresses and bit/masks used by the driver. >>> >>> Signed-off-by: Stanimir Varbanov >>> --- >>> drivers/crypto/qce/regs-v5.h | 327 >>> +++ >>> 1 file changed, 327 insertions(+) >>> create mode 100644 drivers/crypto/qce/regs-v5.h >> >> This should be part of the first patch in the series, you shouldn’t have any >> patch that doesn’t build. > > Correct, I thought I mentioned in the cover letter that the series is > splitted by files to easier the review. I planned to squash all in one > patch after review is done. skipped the cover letter ;). Normally I think we’d tag this with a [RFC] than on the subject line. - k -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] crypto: qce: Add register defines
Minor nitpicks. On 03/04/14 17:17, Stanimir Varbanov wrote: +#define REQ_SIZE 17 /* bit 20-17 */ I would have defined macros like this with _SHIFT at the end, something like this: #define REQ_SIZE_SHIFT 17 +#define REQ_SIZE_MASK (0xf << REQ_SIZE) You could possibly use GENMASK macro for this, its much readable, in *some cases* it could reduce few more lines in header too. #define REQ_SIZE_MASK GENMASK(20, 17) My comments are equally applicable to most macros in this header file. -- srini Linaro Qualcomm Landing Team. Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] crypto: qce: Add register defines
Hi Srini, Thanks for the comments. On 04/04/2014 12:23 PM, Srinivas Kandagatla wrote: > > Minor nitpicks. > > On 03/04/14 17:17, Stanimir Varbanov wrote: >> +#define REQ_SIZE17 /* bit 20-17 */ > > I would have defined macros like this with _SHIFT at the end, something > like this: > > #define REQ_SIZE_SHIFT 17 sure, I will add _SHIFT suffix to all places where it is applicable. > >> +#define REQ_SIZE_MASK(0xf << REQ_SIZE) > You could possibly use GENMASK macro for this, its much readable, in > *some cases* it could reduce few more lines in header too. > > #define REQ_SIZE_MASKGENMASK(20, 17) > > > My comments are equally applicable to most macros in this header file. yes, right will do. Thanks. regards, Stan -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
