From: Tom Rini <trini@konsulko.com>
To: Lokesh Vutla <lokeshvutla@ti.com>
Cc: Tero Kristo <kristo@kernel.org>, u-boot@lists.denx.de
Subject: Re: [PATCHv6 00/26] HSM rearch series for TI K3 devices
Date: Fri, 11 Jun 2021 10:13:17 -0400 [thread overview]
Message-ID: <20210611141317.GV9516@bill-the-cat> (raw)
In-Reply-To: <4d3f8b0d-0ad1-6d39-65b2-6a0599916667@ti.com>
[-- Attachment #1: Type: text/plain, Size: 4148 bytes --]
On Fri, Jun 11, 2021 at 07:13:19PM +0530, Lokesh Vutla wrote:
> Tom,
>
> On 11/06/21 4:48 pm, Tero Kristo wrote:
> > On 11/06/2021 14:08, Lokesh Vutla wrote:
> >> Hi Tero,
> >>
> >> On 11/06/21 2:15 pm, Tero Kristo wrote:
> >>> Hello,
> >>>
> >>> One more post, this time with the #ifdef hackery converted to use the
> >>> IS_ENABLED / CONFIG_IS_ENABLED macros, and also removed the "common.h"
> >>> include from k3-clk.h header. This version also contains fixes to any
> >>> build issues reported by Lokesh, and these are squashed in to relevant
> >>> patches.
> >>
> >> Can you see if the below warnings can be fixed?
> >>
> >> hsm/0018-arm-mach-k3-add-support-for-detecting-firmware-image.patch
> >> -------------------------------------------------------------------
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #26: FILE: arch/arm/mach-k3/common.c:31:
> >> +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
> >
> > This is static data, can't be fixed. Unless we want to compile it in always?
> >
> >>
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #35: FILE: arch/arm/mach-k3/common.c:40:
> >> +#if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
> >
> > Same, static data.
> >
> >>
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #55: FILE: arch/arm/mach-k3/common.c:131:
> >> +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
> >
> > This is actually just an old macro I changed from #ifdef to IS_ENABLED. Fixing
> > the whole file from the existing #ifdef:s should be outside the scope of this
> > series.
> >
> >>
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #124: FILE: arch/arm/mach-k3/common.c:264:
> >> +#if CONFIG_IS_ENABLED(FIT_IMAGE_POST_PROCESS)
> >
> > This code addresses the static data defined before, changing this will break
> > compilation; unless we compile the data always in.
> >
> >>
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #128: FILE: arch/arm/mach-k3/common.c:268:
> >> +#if IS_ENABLED(CONFIG_SYS_K3_SPL_ATF)
> >
> > Same as above.
> >
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #150: FILE: arch/arm/mach-k3/common.c:290:
> >> +#if IS_ENABLED(CONFIG_TI_SECURE_DEVICE)
> >
> > This can't be changed, the code it addresses is only linked in with the config,
> > causing a linker time failure if this is fixed.
> >
> > Imho, I am not too convinced about the checkpatch tool complaining about these
> > issues. :)
> >
> > -Tero
> >
> >>
> >> total: 0 errors, 6 warnings, 0 checks, 144 lines checked
> >>
> >> NOTE: For some of the reported defects, checkpatch may be able to
> >> mechanically convert to the typical style using --fix or --fix-inplace.
> >>
> >> hsm/0018-arm-mach-k3-add-support-for-detecting-firmware-image.patch has style
> >> problems, please review.
> >> -------------------------------------------------------------------
> >> hsm/0019-arm-mach-k3-do-board-config-for-PM-only-if-supported.patch
> >> -------------------------------------------------------------------
> >> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> >> possible
> >> #24: FILE: arch/arm/mach-k3/sysfw-loader.c:162:
> >> +#if !CONFIG_IS_ENABLED(K3_DM_FW)
> >>
> >> total: 0 errors, 1 warnings, 0 checks, 13 lines checked
> >>
> >> NOTE: For some of the reported defects, checkpatch may be able to
> >> mechanically convert to the typical style using --fix or --fix-inplace.
>
> I assume you are okay with this?
Well, if all the cases where we should have been using
IS_ENABLED/CONFIG_IS_ENABLED but weren't have been fixed, and we aren't
adding <common.h> to more files, then yes, everything else that can be
done as a later clean-up can just be done as a later clean-up, and
indeed if it's not possible to use an if (...) it's not possible to use
an if :)
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
prev parent reply other threads:[~2021-06-11 14:13 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 8:45 [PATCHv6 00/26] HSM rearch series for TI K3 devices Tero Kristo
2021-06-11 8:45 ` [PATCHv6 01/26] lib: rational: copy the rational fraction lib routines from Linux Tero Kristo
2021-06-11 8:45 ` [PATCHv6 02/26] arm: mach-k3: introduce new config option for sysfw split Tero Kristo
2021-06-11 8:45 ` [PATCHv6 03/26] remoteproc: k3-r5: remove sysfw PM calls if not supported Tero Kristo
2021-06-11 8:45 ` [PATCHv6 04/26] common: fit: Update board_fit_image_post_process() to pass fit and node_offset Tero Kristo
2021-06-11 8:45 ` [PATCHv6 05/26] clk: fixed_rate: add API for directly registering fixed rate clocks Tero Kristo
2021-06-11 8:45 ` [PATCHv6 06/26] clk: fix clock tree dump to properly dump out every registered clock Tero Kristo
2021-06-11 8:45 ` [PATCHv6 07/26] clk: do not attempt to fetch clock pointer with null device Tero Kristo
2021-06-11 8:45 ` [PATCHv6 08/26] clk: add support for setting clk rate from cmdline Tero Kristo
2021-06-11 8:45 ` [PATCHv6 09/26] clk: sci-clk: fix return value of set_rate Tero Kristo
2021-06-11 8:45 ` [PATCHv6 10/26] clk: fix assigned-clocks to pass with deferring provider Tero Kristo
2021-06-11 8:45 ` [PATCHv6 11/26] clk: fix set_rate to clean up cached rates for the hierarchy Tero Kristo
2021-06-11 8:45 ` [PATCHv6 12/26] clk: add support for TI K3 SoC PLL Tero Kristo
2021-06-11 8:45 ` [PATCHv6 13/26] clk: add support for TI K3 SoC clocks Tero Kristo
2021-06-11 8:45 ` [PATCHv6 14/26] power: domain: Introduce driver for raw TI K3 PDs Tero Kristo
2021-06-11 8:45 ` [PATCHv6 15/26] cmd: ti: pd: Add debug command for K3 power domains Tero Kristo
2021-06-11 8:45 ` [PATCHv6 16/26] tools: k3_fit_atf: add DM binary to the FIT image Tero Kristo
2021-06-11 8:45 ` [PATCHv6 17/26] arm: mach-k3: Add platform data for j721e and j7200 Tero Kristo
2021-06-11 8:45 ` [PATCHv6 18/26] arm: mach-k3: add support for detecting firmware images from FIT Tero Kristo
2021-06-11 8:45 ` [PATCHv6 19/26] arm: mach-k3: do board config for PM only if supported Tero Kristo
2021-06-11 8:45 ` [PATCHv6 20/26] arm: mach-k3: common: Drop main r5 start Tero Kristo
2021-06-11 8:45 ` [PATCHv6 21/26] arm: mach-k3: sysfw-loader: pass boardcfg to sciserver Tero Kristo
2021-06-11 8:45 ` [PATCHv6 22/26] arm: mach-k3: j721e_init: Force early probe of clk-k3 driver Tero Kristo
2021-06-11 8:45 ` [PATCHv6 23/26] configs: j721e_evm_r5: Enable raw access power management features Tero Kristo
2021-06-11 8:45 ` [PATCHv6 24/26] configs: j7200_evm_r5: " Tero Kristo
2021-06-11 8:45 ` [PATCHv6 25/26] board: ti: j72xx: README: update build instructions and image formats Tero Kristo
2021-06-11 8:45 ` [PATCHv6 26/26] arm: dts: k3-j72xx: correct MCU timer1 frequency Tero Kristo
2021-06-11 11:08 ` [PATCHv6 00/26] HSM rearch series for TI K3 devices Lokesh Vutla
2021-06-11 11:18 ` Tero Kristo
2021-06-11 13:43 ` Lokesh Vutla
2021-06-11 14:13 ` Tom Rini [this message]
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=20210611141317.GV9516@bill-the-cat \
--to=trini@konsulko.com \
--cc=kristo@kernel.org \
--cc=lokeshvutla@ti.com \
--cc=u-boot@lists.denx.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox