From: Borislav Petkov <bp@alien8.de>
To: Lei Wang <leiwang_git@outlook.com>
Cc: "james.morse@arm.com" <james.morse@arm.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"mark.rutland@arm.com" <mark.rutland@arm.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"mchehab@kernel.org" <mchehab@kernel.org>,
"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
"sashal@kernel.org" <sashal@kernel.org>,
"hangl@microsoft.com" <hangl@microsoft.com>,
"lewan@microsoft.com" <lewan@microsoft.com>,
"ruizhao@microsoft.com" <ruizhao@microsoft.com>
Subject: Re: [PATCH v5 2/2] EDAC: add EDAC driver for DMC520
Date: Mon, 19 Aug 2019 11:31:47 +0200 [thread overview]
Message-ID: <20190819093147.GE4841@zn.tnic> (raw)
In-Reply-To: <BY5PR04MB659914AC91EBB3EAF72977BD86D20@BY5PR04MB6599.namprd04.prod.outlook.com>
On Tue, Aug 13, 2019 at 01:05:02AM +0000, Lei Wang wrote:
> Added some more comments for the file:
>
> /*
> * dmc520_edac.c, EDAC driver for DMC-520 memory controller
^^^^^^^^^^^^^
Filename is redundant.
> These comments tell how to potentially expand the driver functions to
> support more interrupts besides what are already here.
I can read that - the question is why are they there and for whom? For
your future colleagues who'll take over this driver or what is those
comments' purpose?
> As above comments, this comment is to guide potential future adding to
> this driver to support other interrupts.
See question above.
> After edac_mc_alloc(), if succeeds, the above code updates dmc520_edac
> struct data. If moving edac_mc_alloc as suggested, I will need to use
> local variables to store the data,
Yes, do that pls.
> Do you mean having an array to keep all the irq_id, and then only
> devm_request_irq on them if all of the platform_get_irq are success?
No, move it before edac_mc_alloc().
In general, do *all* initialization of your hardware first and only
then, when it succeeds, allocate the EDAC structures.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
next prev parent reply other threads:[~2019-08-19 9:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 0:49 [PATCH v5 2/2] EDAC: add EDAC driver for DMC520 Lei Wang
2019-08-07 14:40 ` Borislav Petkov
2019-08-13 1:05 ` Lei Wang
2019-08-19 9:31 ` Borislav Petkov [this message]
2019-08-27 1:49 ` Lei Wang
2019-08-27 7:53 ` Borislav Petkov
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=20190819093147.GE4841@zn.tnic \
--to=bp@alien8.de \
--cc=devicetree@vger.kernel.org \
--cc=hangl@microsoft.com \
--cc=james.morse@arm.com \
--cc=leiwang_git@outlook.com \
--cc=lewan@microsoft.com \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mchehab@kernel.org \
--cc=robh+dt@kernel.org \
--cc=ruizhao@microsoft.com \
--cc=sashal@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: link
Be 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.