From: "Shenhar, Talel" <talel@amazon.com> To: Borislav Petkov <bp@alien8.de> Cc: <mchehab@kernel.org>, <james.morse@arm.com>, <davem@davemloft.net>, <gregkh@linuxfoundation.org>, <nicolas.ferre@microchip.com>, <robh+dt@kernel.org>, <mark.rutland@arm.com>, <catalin.marinas@arm.com>, <will@kernel.org>, <linux-edac@vger.kernel.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <dwmw@amazon.co.uk>, <benh@kernel.crashing.org>, <hhhawa@amazon.com>, <ronenk@amazon.com>, <jonnyc@amazon.com>, <hanochu@amazon.com>, <eitan@amazon.com> Subject: Re: [PATCH v6 2/2] EDAC: al-mc-edac: Introduce Amazon's Annapurna Labs Memory Controller EDAC Date: Tue, 5 May 2020 13:55:14 +0300 [thread overview] Message-ID: <4d389169-89d1-4512-f6f0-c98477ab3623@amazon.com> (raw) In-Reply-To: <20200504183716.GJ15046@zn.tnic> On 5/4/2020 9:37 PM, Borislav Petkov wrote: > > On Mon, May 04, 2020 at 01:16:10PM +0300, Shenhar, Talel wrote: >>>> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, >>>> + sizeof(struct al_mc_edac)); >>> You can let that line stick out. >> I rather avoid having this as a checkpatch warnning... (automation and >> stuff...) > checkpatch.pl - while useful - should not be taken to the letter and > human brain should be applied to sanity check it what it warns about. > >> This line break does seems to my eye as too hard to read. >> >> Let me know if you feel strongly about it. > I'm just sayin' - in the end of the day you'll be staring at that code - > not me - so whatever *you* prefer. :-) > > Just don't follow tools blindly. Thanks, I will leave it that way as it will make my life easier (with automatic vim tools and automation) and doesn't really break code understanding. > >>>> + if (al_mc->irq_ue <= 0 || al_mc->irq_ce <= 0) >>> Shouldn't this be && here? >>> >>> I mean, you want to poll when neither of the IRQs can be found. But then >>> if you find one of them and not the other, what do you do? Poll and >>> interrupt? Is that case even possible? >> Correct. >> >> In case dt defined interrupt line only for one type and not for the other, >> than the interrupt mode shall be used for one of them while polling mode for >> the other. > That warrants a comment above it. Shall be part of v7. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
WARNING: multiple messages have this Message-ID (diff)
From: "Shenhar, Talel" <talel@amazon.com> To: Borislav Petkov <bp@alien8.de> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, benh@kernel.crashing.org, hhhawa@amazon.com, gregkh@linuxfoundation.org, eitan@amazon.com, jonnyc@amazon.com, hanochu@amazon.com, linux-kernel@vger.kernel.org, robh+dt@kernel.org, james.morse@arm.com, dwmw@amazon.co.uk, catalin.marinas@arm.com, ronenk@amazon.com, mchehab@kernel.org, will@kernel.org, davem@davemloft.net, linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org Subject: Re: [PATCH v6 2/2] EDAC: al-mc-edac: Introduce Amazon's Annapurna Labs Memory Controller EDAC Date: Tue, 5 May 2020 13:55:14 +0300 [thread overview] Message-ID: <4d389169-89d1-4512-f6f0-c98477ab3623@amazon.com> (raw) In-Reply-To: <20200504183716.GJ15046@zn.tnic> On 5/4/2020 9:37 PM, Borislav Petkov wrote: > > On Mon, May 04, 2020 at 01:16:10PM +0300, Shenhar, Talel wrote: >>>> + mci = edac_mc_alloc(0, ARRAY_SIZE(layers), layers, >>>> + sizeof(struct al_mc_edac)); >>> You can let that line stick out. >> I rather avoid having this as a checkpatch warnning... (automation and >> stuff...) > checkpatch.pl - while useful - should not be taken to the letter and > human brain should be applied to sanity check it what it warns about. > >> This line break does seems to my eye as too hard to read. >> >> Let me know if you feel strongly about it. > I'm just sayin' - in the end of the day you'll be staring at that code - > not me - so whatever *you* prefer. :-) > > Just don't follow tools blindly. Thanks, I will leave it that way as it will make my life easier (with automatic vim tools and automation) and doesn't really break code understanding. > >>>> + if (al_mc->irq_ue <= 0 || al_mc->irq_ce <= 0) >>> Shouldn't this be && here? >>> >>> I mean, you want to poll when neither of the IRQs can be found. But then >>> if you find one of them and not the other, what do you do? Poll and >>> interrupt? Is that case even possible? >> Correct. >> >> In case dt defined interrupt line only for one type and not for the other, >> than the interrupt mode shall be used for one of them while polling mode for >> the other. > That warrants a comment above it. Shall be part of v7. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-05 10:55 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-24 13:41 [PATCH v6 0/2] Amazon's Annapurna Labs Memory Controller EDAC Talel Shenhar 2020-02-24 13:41 ` Talel Shenhar 2020-02-24 13:41 ` [PATCH v6 1/2] dt-bindings: edac: al-mc-edac: " Talel Shenhar 2020-02-24 13:41 ` Talel Shenhar 2020-04-28 11:06 ` Borislav Petkov 2020-04-28 11:06 ` Borislav Petkov 2020-05-03 14:21 ` Shenhar, Talel 2020-05-03 14:21 ` Shenhar, Talel 2020-05-05 10:44 ` Shenhar, Talel 2020-05-05 10:44 ` Shenhar, Talel 2020-05-07 14:44 ` Shenhar, Talel 2020-05-07 14:44 ` Shenhar, Talel 2020-02-24 13:41 ` [PATCH v6 2/2] EDAC: al-mc-edac: Introduce " Talel Shenhar 2020-02-24 13:41 ` Talel Shenhar 2020-04-28 11:39 ` Borislav Petkov 2020-04-28 11:39 ` Borislav Petkov [not found] ` <46ccdb47-f28d-63f7-e759-1ba34e98add8@amazon.com> 2020-05-04 18:37 ` Borislav Petkov 2020-05-04 18:37 ` Borislav Petkov 2020-05-05 10:55 ` Shenhar, Talel [this message] 2020-05-05 10:55 ` Shenhar, Talel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4d389169-89d1-4512-f6f0-c98477ab3623@amazon.com \ --to=talel@amazon.com \ --cc=benh@kernel.crashing.org \ --cc=bp@alien8.de \ --cc=catalin.marinas@arm.com \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=dwmw@amazon.co.uk \ --cc=eitan@amazon.com \ --cc=gregkh@linuxfoundation.org \ --cc=hanochu@amazon.com \ --cc=hhhawa@amazon.com \ --cc=james.morse@arm.com \ --cc=jonnyc@amazon.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-edac@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mchehab@kernel.org \ --cc=nicolas.ferre@microchip.com \ --cc=robh+dt@kernel.org \ --cc=ronenk@amazon.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.