Re: [PATCH 0/2] Compiler Attributes: __fallthrough
On Mon, Oct 22, 2018 at 09:54:27AM -0700, Nick Desaulniers wrote: > On Mon, Oct 22, 2018 at 2:48 AM Miguel Ojeda > wrote: > > > > On Sun, Oct 21, 2018 at 8:29 PM Greg Kroah-Hartman > > wrote: > > > > > > Will this work with all of the static tools that are currently looking > > > for the comment instead? I know coverity handles that, what about > > > others? > > > > I will also contact the different tools about this. > > Let's contact the authors of these tools if they don't parse the > attribute. I prefer to have the attributes rather than specifically > formatted comments. > > I do think this may be tricky to provide backwards support for though; > Miguel, do you have info on which versions of GCC support comments vs > attribute? None. GCC 7.1 added support for the warning, the comment parsing and the attribute so it's fine. The only thing that we know for sure is an issue is Eclipse. We need to test Coverity but it should work in theory. And we don't know about CPPcheck. regards, dan carpenter
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 7:50 PM Bernd Petrovitsch wrote: > > Hi all! > > On 22/10/18 13:07, Miguel Ojeda wrote: > > On Mon, Oct 22, 2018 at 12:54 PM Dan Carpenter > > wrote: > >> > >> Doing both is super ugly. Let's just do comments until Eclipse gets > >> updated. > > Yes, "Eclipse" as the IDE. > > And yes but IMHO better super ugly than loosing the warning - YMMV. I think Dan meant too simply not touch anything (i.e. not losing the warning anywhere). > > For the archives: I have Eclipse Photon/June 2016 here. And "no break" > is the (default) string in a comment used by Eclipse (it can be > customized and is actually a regexp but it must be in a comment). > Hm... that means they don't support (by default) the same regexps as GCC; which is bad: it means that it is only equivalent to the most relaxed level in gcc, 1: """ -Wimplicit-fallthrough=1 matches .* regular expression, any comment is used as fallthrough comment. """ See https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html i.e. any other level above will either not match Eclipse or not match gcc. > >> I had wanted to move to the attribute because that would simplify things > >> in Smatch but it's not a huge deal to delay for another year. > > > > I can re-send them later on, no problem. On the other hand, doing the > > changes will push tools to get updated sooner ;-) > > > > If tools were doing something as fancy as comment parsing for > > diagnostics, they should have been updated with the attribute support > > (either gcc's or C++17's) -- it has been more than a year now since > > gcc 7.1 and the C++17 final draft. (Note that this does not apply for > > things like clang, since they weren't doing comment parsing to begin > > with.) > > That would be nice. And if they agree on the same texts (or accept per > default all somewhat widely used and/or old ones). > > After stumbling over > https://stackoverflow.com/questions/16935935/how-do-i-turn-off-a-static-code-analysis-warning-on-a-line-by-line-warning-in-cd, > looking into Eclipses Window -> Preferences -> C/C++ -> Code Analysis -> > "No break at the end of case" screen (that's the screenshot there) and I > tried various things: > > Preface: > I have > snip > #define __fallthrough __attribute__((fallthrough)) > snip > for gcc >= 7 (because clang doesn't know it and I had also older > gcc's in use before). > > So: > - Adding a comment to the #define doesn't change anything for Eclipse. It shouldn't according to the standard -- but who knows... :-) > - Eclipse looks *only* in comments for the string/regexp given > the warnings configuration (and that comment must be on the line > directly before the "case"). > - Eclipse understands [[fallthrough]] out-of-the-box though (which > is C++11 AFAIK) as does g++-7 (I use -std=gnu++17 - most of the > sources are C++, but not all) and clang++-6 (all the current standard > Ubuntu-18.06/Bionic packages). Eclipse understanding [[fallthrough]] is very good, actually. > Eclipse "accepts" [[fallthrough]] only in C++ sources (and not in C > sources). Bad, but I guess they will add it to C eventually, since it is probably coming for C2x. > - Neither gcc nor clang understand [[fallthrough]] (so it's probably a > no-go for the Kernel with C89 anyways). clang does it if you enable -fdouble-square-bracket-attributes (please see my other messages). gcc will at some point (if C2x gets the attributes), but at the moment the C parser is different than the C++ parser and there is no support for it on trunk that I could see, so they will have to copy the support; i.e. it will take a bit more time than clang, likely. Thanks a *lot* for taking a look at Eclipse! Cheers, Miguel
[PATCH] Documentation/proc.txt: Add 2 missing fields for /proc//status
It was found that two of the fields in the /proc//status file were missing - CapAmb & Speculation_Store_Bypass. They are now added to the proc.txt documentation file. Signed-off-by: Waiman Long --- Documentation/filesystems/proc.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt index 22b4b00..36a9e62 100644 --- a/Documentation/filesystems/proc.txt +++ b/Documentation/filesystems/proc.txt @@ -214,7 +214,7 @@ asynchronous manner and the value may not be very precise. To see a precise snapshot of a moment, you can see /proc//smaps file and scan page table. It's slow but very precise. -Table 1-2: Contents of the status files (as of 4.8) +Table 1-2: Contents of the status files (as of 4.18) .. Field Content Namefilename of the executable @@ -267,8 +267,10 @@ Table 1-2: Contents of the status files (as of 4.8) CapPrm bitmap of permitted capabilities CapEff bitmap of effective capabilities CapBnd bitmap of capabilities bounding set + CapAmb bitmap of ambient capabilities NoNewPrivs no_new_privs, like prctl(PR_GET_NO_NEW_PRIV, ...) Seccomp seccomp mode, like prctl(PR_GET_SECCOMP, ...) + Speculation_Store_Bypassspeculative store bypass mitigation status Cpus_allowedmask of CPUs on which this process may run Cpus_allowed_list Same as previous, but in "list format" Mems_allowedmask of memory nodes allowed to this process -- 1.8.3.1
Re: [PATCH 0/2] Compiler Attributes: __fallthrough
On Mon, Oct 22, 2018 at 6:54 PM Nick Desaulniers wrote: > > On Mon, Oct 22, 2018 at 2:48 AM Miguel Ojeda > wrote: > > > > On Sun, Oct 21, 2018 at 8:29 PM Greg Kroah-Hartman > > wrote: > > > > > > Will this work with all of the static tools that are currently looking > > > for the comment instead? I know coverity handles that, what about > > > others? > > > > I will also contact the different tools about this. > > Let's contact the authors of these tools if they don't parse the > attribute. I prefer to have the attributes rather than specifically > formatted comments. Sorry, not sure what you mean -- isn't that what I said? Greg was asking whether tools would support the attribute equally well compared to the comment parsing; not the comments. > > I do think this may be tricky to provide backwards support for though; > Miguel, do you have info on which versions of GCC support comments vs > attribute? It is in the commit message: """ In C mode, GCC supports the __fallthrough__ attribute since 7.1, the same time the warning and the comment parsing were introduced. """ Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 7:36 PM Nick Desaulniers wrote: > > On Sun, Oct 21, 2018 at 10:14 AM Miguel Ojeda > wrote: > > > > of its kind) to deal with this: [[fallthrough]] is meant to be > > a new control keyword in the form of an extension. > > I think we can leave the details about the [[]] notation out. IIUC, > it's only applicable to C++. No, because C++ is the driving force for the standard attributes syntax; i.e. C2x is adding them because of the syntax published by C++17; and possibly gcc 7.1 added support for fallthrough (and comment parsing) due to C++17 too. Basically, it is a small paragraph explaining how this came to be. > > +#if __has_attribute(__fallthrough__) > > +# define __fallthrough __attribute__((__fallthrough__)) > > +#else > > +# define __fallthrough > > While this is in the correct format as the other attributes in this > file, I think this particular attribute is a special case, because of > the variety of fallbacks and differing support for them. I'd like to No, is it the correct format because we cannot add support for the other syntax in gcc; so the best way to proceed is to simply wait until clang supports the GNU attribute in C mode. The tooling, of course, is another matter and independent of this. > see in the commit message maybe a list of tools we'd like to support Yes, I already said I would write it in one of the other threads. > and links to the feature requests/bug reports for them. I acknowledge > it's more work to file bugs, but it's being a good open source > citizen, IMO. Who said we aren't going to do it? :-) I was actually in the process of checking which OSS tools supported what and how easy it was to fix in each of them (gcc's [[...]] syntax, clang's GNU and C++ attrs in C mode, cppcheck's fallthrough support...), but it takes time; I prefer to do the research beforehand; so that the submitted bug reports are better/more precise/more helpful, etc. However, you already sent the LLVM report (without mentioning this thread or me, nor the -fdouble-square-bracket-attributes clang flag that I mentioned). That is a bit rude :-) Please take things a little easier, there is no need to rush stuff. If I didn't have submitted the LLVM bug report is because I hadn't finish looking at the issue. In general, I think it is polite (and also more productive to avoid duplicating efforts) to first ask whoever is working on something before you rush to do it... > > I'm also curious which is the first version of GCC to support the > comment format? gcc 7.1 started everything. It is stated in the commit message and several messages/threads already. Again, please pause, relax and read a bit before sending stuff around :-) Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 7:17 PM Nick Desaulniers wrote: > > On Mon, Oct 22, 2018 at 2:43 AM Miguel Ojeda > wrote: > > > > On Mon, Oct 22, 2018 at 11:41 AM Miguel Ojeda > > wrote: > > > > > > * clang does *not* support the attribute in C. > > > > ...nor the warning at all, by the way (which is why this is OK). > > > > Cheers, > > Miguel > > Oh? We're going through and annotating all of Android's C++ source > right now with it. > https://clang.llvm.org/docs/DiagnosticsReference.html#wimplicit-fallthrough > Sure, but I am talking about the C mode only. Please read the previous messages in the thread :-) > Though it looks like the attribute syntax I like from GCC is not yet > supported in Clang. Indeed, that is what I explained in the last message. > https://bugs.llvm.org/show_bug.cgi?id=37135 > https://github.com/ClangBuiltLinux/linux/issues/235 > I'll take a look at adding support. > > So Kees sent me this embarrassing example: > https://godbolt.org/z/gV_c9_ > (try changing the language from C++ to C; it works)! That's embarrassing! Yup, that is what I have been testing yesterday -- see the linked thread in the commit message for the summary of the results. > https://bugs.llvm.org/show_bug.cgi?id=39382 > https://github.com/ClangBuiltLinux/linux/issues/236 > Will get these both fixed up. Cheers, Miguel
Re: [PATCH v9 4/9] dt-bindings: i3c: Document core bindings
On Mon, Oct 22, 2018 at 03:33:59PM +0200, Boris Brezillon wrote:
> A new I3C subsystem has been added and a generic description has been
> created to represent the I3C bus and the devices connected on it.
>
> Document this generic representation.
>
> Cc: Rob Herring
> Signed-off-by: Boris Brezillon
> ---
> Rob,
>
> Can you have a look at this v9. I know you previously acked this
> binding but I changed a bit the way we declare I2C devices and thus
> had to drop your Ack.
> The IS_I2C_DEV bit no long exists, and we instead consider that any
> device with the 2nd reg entry set to 0 is an I2C device, which is
> just fine because this entry is supposed to contain the upper part of
> the Provisional ID which is basically the MIPI manufacturer ID,
> and 0 is not a valid manufacturer ID.
>
> By doing that we make reg definition more straightforward and can
> drop the ugly {I2C,I3C}_DEV() macros.
>
> Regards,
>
> Boris
>
> Changes in v9:
> - Rework the way we encode the I2C vs I3C device in the reg property
> so that we don't need the funky macros to define and I3C or I2C dev
> - Drop Rob's Reviewed-by
>
> Changes in v8:
> - None
>
> Changes in v7:
> - None
>
> Changes in v6:
> - None
>
> Changes in v5:
> - Add Rob's R-b
>
> Changes in v4:
> - Clarify the fact that static address == I3C address and dynamic
> address == I3C address
> - Use i2c-scl-hz in the example
>
> Changes in v3:
> - Rename {i2c,i3c}-scl-frequency DT prop into {i2c,i3c}-scl-hz
> - Rework the way we expose the provisional ID and LVR information
> - Rename dynamic-address into assigned-address
> - Enforce the I3C master node name
>
> Changes in v2:
> - Define how to describe I3C devices in the DT and when it should be
> used. Note that the parsing of I3C devices is not yet implemented in
> the framework. Will be added when someone really needs it.
> ---
> Documentation/devicetree/bindings/i3c/i3c.txt | 138 ++
> 1 file changed, 138 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
>
> diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt
> b/Documentation/devicetree/bindings/i3c/i3c.txt
> new file mode 100644
> index ..7021468e0205
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/i3c.txt
> @@ -0,0 +1,138 @@
> +Generic device tree bindings for I3C busses
> +===
> +
> +This document describes generic bindings that should be used to describe I3C
> +busses in a device tree.
> +
> +Required properties
> +---
> +
> +- #address-cells - should be <3>. Read more about addresses below.
> +- #size-cells - should be <0>.
> +- compatible - name of the I3C master controller driving the I3C bus
> +
> +For other required properties e.g. to describe register sets,
> +clocks, etc. check the binding documentation of the specific driver.
> +The node describing an I3C bus should be named i3c-master.
> +
> +Optional properties
> +---
> +
> +These properties may not be supported by all I3C master drivers. Each I3C
> +master bindings should specify which of them are supported.
> +
> +- i3c-scl-hz: frequency of the SCL signal used for I3C transfers.
> + When undefined the core sets it to 12.5MHz.
> +
> +- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
> + When undefined, the core looks at LVR (Legacy Virtual Register)
> + values of I2C devices described in the device tree to determine
> + the maximum I2C frequency.
> +
> +I2C devices
> +===
> +
> +Each I2C device connected to the bus should be described in a subnode. All
> +properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
> +valid here, but several new properties have been added.
> +
> +New constraint on existing properties:
> +--
> +- reg: contains 3 cells
> + + first cell : still encoding the I2C address
> +
> + + second cell: shall be 0
> +
> + + third cell: shall encode the I3C LVR (Legacy Virtual Register)
> + bit[31:8]: unused/ignored
> + bit[7:5]: I2C device index. Possible values
> + * 0: I2C device has a 50 ns spike filter
> + * 1: I2C device does not have a 50 ns spike filter but supports high
> + frequency on SCL
> + * 2: I2C device does not have a 50 ns spike filter and is not tolerant
> + to high frequencies
> + * 3-7: reserved
> +
> + bit[4]: tell whether the device operates in FM (Fast Mode) or FM+ mode
> + * 0: FM+ mode
> + * 1: FM mode
> +
> + bit[3:0]: device type
> + * 0-15: reserved
> +
> +I3C devices
> +===
> +
> +All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
> +are thus discoverable. So, by default, I3C devices do not have to be
> described
> +in the device tree.
> +This being said, one might want to attach extra resources to these devices,
> +and those resources may have to be described
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 11:15 AM Bernd Petrovitsch
wrote:
>
> Hi all!
>
> On 22/10/18 19:54, Nick Desaulniers wrote:
> > On Mon, Oct 22, 2018 at 10:50 AM Bernd Petrovitsch
> > wrote:
> [...]
> >> PS: clang++ errors with "fallthrough annotation in unreachable code" if
> >> [[fallthrough]] is after an assert(). clang-devs there, please, the
> >> fallthrough doesn't really generated code (I hope;-).
> [...]
> > Can you send me a link to a simple reproducer in godbolt (godbolt.org)
> > and we'll take a look?
>
> Does https://godbolt.org/z/2Y4zIo do it - I'm a godbolt-newbie?
Moving the kernel folks to bcc, since we don't need to be discussing
C++ on LKML.
https://godbolt.org/z/B1fo9Z shows that this works as intended, for
cases that cannot be statically proven. I guess I'm looking for a
more realistic code sample to show why putting a `break;` statement
there is untenable?
>
> For
> snip
> #include
>
> int main(void)
> {
> switch (1) {
> default:
> assert(0);
> [[fallthrough]];
> case 1:
> ;
> }
> return 0;
> }
> snip
> Just "clang++ -Wimplicit-fallthrough -Werror" it .
>
> MfG,
> Bernd
> --
> "I dislike type abstraction if it has no real reason. And saving
> on typing is not a good reason - if your typing speed is the main
> issue when you're coding, you're doing something seriously wrong."
> - Linus Torvalds
--
Thanks,
~Nick Desaulniers
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
Hi all!
On 22/10/18 19:54, Nick Desaulniers wrote:
> On Mon, Oct 22, 2018 at 10:50 AM Bernd Petrovitsch
> wrote:
[...]
>> PS: clang++ errors with "fallthrough annotation in unreachable code" if
>> [[fallthrough]] is after an assert(). clang-devs there, please, the
>> fallthrough doesn't really generated code (I hope;-).
[...]
> Can you send me a link to a simple reproducer in godbolt (godbolt.org)
> and we'll take a look?
Does https://godbolt.org/z/2Y4zIo do it - I'm a godbolt-newbie?
For
snip
#include
int main(void)
{
switch (1) {
default:
assert(0);
[[fallthrough]];
case 1:
;
}
return 0;
}
snip
Just "clang++ -Wimplicit-fallthrough -Werror" it .
MfG,
Bernd
--
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
- Linus Torvalds
pEpkey.asc
Description: application/pgp-keys
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 10:50 AM Bernd Petrovitsch wrote: > > Hi all! > > On 22/10/18 13:07, Miguel Ojeda wrote: > > On Mon, Oct 22, 2018 at 12:54 PM Dan Carpenter > > wrote: > >> > >> Doing both is super ugly. Let's just do comments until Eclipse gets > >> updated. > > Yes, "Eclipse" as the IDE. > > And yes but IMHO better super ugly than loosing the warning - YMMV. > > For the archives: I have Eclipse Photon/June 2016 here. And "no break" > is the (default) string in a comment used by Eclipse (it can be > customized and is actually a regexp but it must be in a comment). > > >> I had wanted to move to the attribute because that would simplify things > >> in Smatch but it's not a huge deal to delay for another year. > > > > I can re-send them later on, no problem. On the other hand, doing the > > changes will push tools to get updated sooner ;-) > > > > If tools were doing something as fancy as comment parsing for > > diagnostics, they should have been updated with the attribute support > > (either gcc's or C++17's) -- it has been more than a year now since > > gcc 7.1 and the C++17 final draft. (Note that this does not apply for > > things like clang, since they weren't doing comment parsing to begin > > with.) > > That would be nice. And if they agree on the same texts (or accept per > default all somewhat widely used and/or old ones). > > After stumbling over > https://stackoverflow.com/questions/16935935/how-do-i-turn-off-a-static-code-analysis-warning-on-a-line-by-line-warning-in-cd, > looking into Eclipses Window -> Preferences -> C/C++ -> Code Analysis -> > "No break at the end of case" screen (that's the screenshot there) and I > tried various things: > > Preface: > I have > snip > #define __fallthrough __attribute__((fallthrough)) > snip > for gcc >= 7 (because clang doesn't know it and I had also older > gcc's in use before). > > So: > - Adding a comment to the #define doesn't change anything for Eclipse. > - Eclipse looks *only* in comments for the string/regexp given > the warnings configuration (and that comment must be on the line > directly before the "case"). > - Eclipse understands [[fallthrough]] out-of-the-box though (which > is C++11 AFAIK) as does g++-7 (I use -std=gnu++17 - most of the > sources are C++, but not all) and clang++-6 (all the current standard > Ubuntu-18.06/Bionic packages). > Eclipse "accepts" [[fallthrough]] only in C++ sources (and not in C > sources). > - Neither gcc nor clang understand [[fallthrough]] (so it's probably a > no-go for the Kernel with C89 anyways). > > MfG, > Bernd > > PS: clang++ errors with "fallthrough annotation in unreachable code" if > [[fallthrough]] is after an assert(). clang-devs there, please, the > fallthrough doesn't really generated code (I hope;-). > I have lots of switch()es which catch undefined values (for enums > et. al.) with "default"+assert() and fall through to the most safe > case (for the deployed version). Can you send me a link to a simple reproducer in godbolt (godbolt.org) and we'll take a look? > -- > "I dislike type abstraction if it has no real reason. And saving > on typing is not a good reason - if your typing speed is the main > issue when you're coding, you're doing something seriously wrong." > - Linus Torvalds -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
Hi all! On 22/10/18 13:07, Miguel Ojeda wrote: > On Mon, Oct 22, 2018 at 12:54 PM Dan Carpenter > wrote: >> >> Doing both is super ugly. Let's just do comments until Eclipse gets >> updated. Yes, "Eclipse" as the IDE. And yes but IMHO better super ugly than loosing the warning - YMMV. For the archives: I have Eclipse Photon/June 2016 here. And "no break" is the (default) string in a comment used by Eclipse (it can be customized and is actually a regexp but it must be in a comment). >> I had wanted to move to the attribute because that would simplify things >> in Smatch but it's not a huge deal to delay for another year. > > I can re-send them later on, no problem. On the other hand, doing the > changes will push tools to get updated sooner ;-) > > If tools were doing something as fancy as comment parsing for > diagnostics, they should have been updated with the attribute support > (either gcc's or C++17's) -- it has been more than a year now since > gcc 7.1 and the C++17 final draft. (Note that this does not apply for > things like clang, since they weren't doing comment parsing to begin > with.) That would be nice. And if they agree on the same texts (or accept per default all somewhat widely used and/or old ones). After stumbling over https://stackoverflow.com/questions/16935935/how-do-i-turn-off-a-static-code-analysis-warning-on-a-line-by-line-warning-in-cd, looking into Eclipses Window -> Preferences -> C/C++ -> Code Analysis -> "No break at the end of case" screen (that's the screenshot there) and I tried various things: Preface: I have snip #define __fallthrough __attribute__((fallthrough)) snip for gcc >= 7 (because clang doesn't know it and I had also older gcc's in use before). So: - Adding a comment to the #define doesn't change anything for Eclipse. - Eclipse looks *only* in comments for the string/regexp given the warnings configuration (and that comment must be on the line directly before the "case"). - Eclipse understands [[fallthrough]] out-of-the-box though (which is C++11 AFAIK) as does g++-7 (I use -std=gnu++17 - most of the sources are C++, but not all) and clang++-6 (all the current standard Ubuntu-18.06/Bionic packages). Eclipse "accepts" [[fallthrough]] only in C++ sources (and not in C sources). - Neither gcc nor clang understand [[fallthrough]] (so it's probably a no-go for the Kernel with C89 anyways). MfG, Bernd PS: clang++ errors with "fallthrough annotation in unreachable code" if [[fallthrough]] is after an assert(). clang-devs there, please, the fallthrough doesn't really generated code (I hope;-). I have lots of switch()es which catch undefined values (for enums et. al.) with "default"+assert() and fall through to the most safe case (for the deployed version). -- "I dislike type abstraction if it has no real reason. And saving on typing is not a good reason - if your typing speed is the main issue when you're coding, you're doing something seriously wrong." - Linus Torvalds pEpkey.asc Description: application/pgp-keys
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Sun, Oct 21, 2018 at 10:14 AM Miguel Ojeda wrote: > > From the GCC manual: > > fallthrough > > The fallthrough attribute with a null statement serves as a > fallthrough statement. It hints to the compiler that a statement > that falls through to another case label, or user-defined label > in a switch statement is intentional and thus the -Wimplicit-fallthrough > warning must not trigger. The fallthrough attribute may appear > at most once in each attribute list, and may not be mixed with > other attributes. It can only be used in a switch statement > (the compiler will issue an error otherwise), after a preceding > statement and before a logically succeeding case label, > or user-defined label. > > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html > > Currently, most of the kernel uses fallthrough comments to silence > the -Wimplicit-fallthrough warnings (enabled by -Wextra, in W=1). > However, C++17 standarized an "statement attribute" (the first s/standarized/standardized/ > of its kind) to deal with this: [[fallthrough]] is meant to be > a new control keyword in the form of an extension. I think we can leave the details about the [[]] notation out. IIUC, it's only applicable to C++. > > In C mode, GCC supports the __fallthrough__ attribute since 7.1, > the same time the warning and the comment parsing were introduced. > > While comment parsing is a good idea to deal with old codebases > that used such a comment as documentation for humans, the best > solution is to use the attribute: > > * It is a "real" part of the AST (=> better for tooling). +1 > > * It does not follow arbitrary rules for parsing (e.g. regexps > for the comment parsing). +2 > > * It may even become standarized in C as well: there are ongoing s/standarized/standardized/ > proposals to import some C++ standard attributes into > the C standard, e.g. for fallthrough: > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf > > On top of that, it is also a better solution for the kernel, because: > > * We can actually use a #define for it like for the rest of > attributes/extensions, which is not possible with a comment, > so that its naming/usage is consistent across the entire kernel. +3 > > * Whenever the migration from the comments to the attribute > is complete, we may increase the level of the GCC warning up to 5, > i.e. comments will not longer be considered for warning > surpression: only the attribute must be used. This would enforce s/surpression/suppression/ > consistency by leveraging the compiler directly (instead of > enforcing it with other tools). > > * Further into the future, we can consider moving the warning > up to W=0 or even making it an error. > > It is worth noting that clang >= 3.2 supports the warning and > the attribute, but only in C++ mode (and it is not enabled by > -Wall/-Wextra/-Wpedantic like in gcc). Hopefully, they will also > support it for C as well. https://bugs.llvm.org/show_bug.cgi?id=39382 > > Further, icc >= 18 does not seem to know anything about the warning; > except that it accepts (i.e. ignores) [[fallthrough]] in C++17 mode > (to be conformant, probably). > > Link: https://lore.kernel.org/lkml/20181017062255.oiu44y4zuuwilan3@mwanda/ > Suggested-by: Dan Carpenter > Signed-off-by: Miguel Ojeda > --- > include/linux/compiler_attributes.h | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/include/linux/compiler_attributes.h > b/include/linux/compiler_attributes.h > index 6b28c1b7310c..9e2153f85462 100644 > --- a/include/linux/compiler_attributes.h > +++ b/include/linux/compiler_attributes.h > @@ -32,6 +32,7 @@ > # define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9) > # define __GCC4_has_attribute___designated_init__ 0 > # define __GCC4_has_attribute___externally_visible__ 1 > +# define __GCC4_has_attribute___fallthrough__ 0 > # define __GCC4_has_attribute___noclone__ 1 > # define __GCC4_has_attribute___optimize__1 > # define __GCC4_has_attribute___nonstring__ 0 > @@ -133,6 +134,23 @@ > # define __visible > #endif > > +/* > + * Currently, most of the kernel uses fallthrough comments to silence > + * the -Wimplicit-fallthrough warnings (enabled by -Wextra, in W=1). > + * For new instances, please use this attribute instead. > + * > + * Optional: only supported since gcc >= 7.1 > + * Optional: not supported by clang > + * Optional: not supported by icc > + * > + * gcc: > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#index-fallthrough-statement-attribute > + */ > +#if __has_attribute(__fallthrough__) > +# define __fallthrough __attribute__((__fallthrough__)) > +#else > +# define __fallthrough While this is in the correct format as the other attributes in this file, I think this particular attribute is a special case, b
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 3:54 AM Dan Carpenter wrote: > > On Mon, Oct 22, 2018 at 12:45:03PM +0200, Bernd Petrovitsch wrote: > > It's more like > > snip > > case 3: > > frob(); > > __fall_through; > > /* no break - fall through */ > > case 4: > > snip > > as "eclipse" doesn't accept anything else. > > > > And yes, it's far from "beautiful" but I hadn't time to dig into > > eclipses innards to fix that. > > > > Doing both is super ugly. Let's just do comments until Eclipse gets > updated. Eclipse as in the IDE? https://bugs.eclipse.org/bugs/ > > I had wanted to move to the attribute because that would simplify things > in Smatch but it's not a huge deal to delay for another year. > > regards, > dan carpenter > -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:26 AM Miguel Ojeda wrote: > > On Mon, Oct 22, 2018 at 12:27 AM Theodore Y. Ts'o wrote: > > > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > > From the GCC manual: > > > > > > fallthrough > > > > > > The fallthrough attribute with a null statement serves as a > > > fallthrough statement. It hints to the compiler that a statement > > > that falls through to another case label, or user-defined label > > > in a switch statement is intentional and thus the > > > -Wimplicit-fallthrough > > > warning must not trigger. The fallthrough attribute may appear > > > at most once in each attribute list, and may not be mixed with > > > other attributes. It can only be used in a switch statement > > > (the compiler will issue an error otherwise), after a preceding > > > statement and before a logically succeeding case label, > > > or user-defined label. > > > > > > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html > > > > Do we know if coverity understands the fallthrough attribute? One of > > the reasons why I started using /* fallthrough */ is because it kept > > Coverity happy. > > If Coverity is like gcc, they should be doing both (i.e. I see the > comment parsing as an "extra" that gcc did, but the "basic stuff" is > the attribute -- and I would guess it is way easier for them to > support than the comment parsing). > > But I cannot test it myself :-( Someone, please? + Colin, who has been running Coverity on the kernel, and sending patches. > > However, if I understood Greg correctly in his reply to the cover > letter, he replied that Coverity knows about it (?). > > > > > If the conversion from /* fallthrough */ to the __fallthrough__ > > attribute means that we start gethting a lot of Coverity warnings, > > that would be unfortunate. OTOH, if this is getting standardized, > > maybe we can get Coverity to understand this attribute? > > Indeed! That would be the best for everyone, including Coverity customers. > > Cheers, > Miguel -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:43 AM Miguel Ojeda wrote: > > On Mon, Oct 22, 2018 at 11:41 AM Miguel Ojeda > wrote: > > > > * clang does *not* support the attribute in C. > > ...nor the warning at all, by the way (which is why this is OK). > > Cheers, > Miguel Oh? We're going through and annotating all of Android's C++ source right now with it. https://clang.llvm.org/docs/DiagnosticsReference.html#wimplicit-fallthrough Though it looks like the attribute syntax I like from GCC is not yet supported in Clang. https://bugs.llvm.org/show_bug.cgi?id=37135 https://github.com/ClangBuiltLinux/linux/issues/235 I'll take a look at adding support. So Kees sent me this embarrassing example: https://godbolt.org/z/gV_c9_ (try changing the language from C++ to C; it works)! That's embarrassing! https://bugs.llvm.org/show_bug.cgi?id=39382 https://github.com/ClangBuiltLinux/linux/issues/236 Will get these both fixed up. -- Thanks, ~Nick Desaulniers
Re: [PATCH 0/2] Compiler Attributes: __fallthrough
On Mon, Oct 22, 2018 at 2:48 AM Miguel Ojeda wrote: > > On Sun, Oct 21, 2018 at 8:29 PM Greg Kroah-Hartman > wrote: > > > > Will this work with all of the static tools that are currently looking > > for the comment instead? I know coverity handles that, what about > > others? > > I will also contact the different tools about this. Let's contact the authors of these tools if they don't parse the attribute. I prefer to have the attributes rather than specifically formatted comments. I do think this may be tricky to provide backwards support for though; Miguel, do you have info on which versions of GCC support comments vs attribute? -- Thanks, ~Nick Desaulniers
Re: [PATCH 2/2] Compiler Attributes: auxdisplay: panel: use __fallthrough
On Mon, 2018-10-22 at 11:51 +0200, Miguel Ojeda wrote: > On Sun, Oct 21, 2018 at 8:11 PM Joe Perches wrote: > > On Sun, 2018-10-21 at 19:14 +0200, Miguel Ojeda wrote: > > > Let gcc know these cases are meant to fall through to the next label > > > by annotating them with the new __fallthrough statement attribute; > > > and remove the comment since it conveys the same information > > > (which was also parsed by gcc to suppress the warning). > > > > Instead of many individual conversion patches, > > perhaps a script to do all the conversions at once. > > Note that this was only an example of the attribute (some people asked > for an example when introducing another one, so I preemptively did it > for this one). > > Doing the conversion (and how :-) I left it for afterwards, if people > agree with the attribute. > > > Maybe: > > > > pattern='(?:\/\*\s*fall(?:\s*|\s*\-\s*)thr(?:u|ough)\s*\*\/|\/\/\s*fall\s*thr(?:u|ough)\s*$)' > > git grep -P -i --name-only "$pattern" -- '*.[ch]' | > > xargs perl -p -i -e "s/$pattern/__fallthrough;/gi" > > By the way, I checked first if coccinelle could match input comments, > but it doesn't, according to Julia. I am also thinking whether a > compiler plugin could easily do this, but I don't have my hopes high > given these are comments... > > Also, regardless of how it is done, the patches need to be sent > individually to maintainers, no? Not really no. For instance the spdx license treewide change. commit b24413180f5600bcb3bb70fbed5cf186b60864bd > I have a vague memory that big & > automated conversions were a bit frozen upon in the kernel. Greg...? There really needs to be some method to do treewide conversions without involving multiple maintainers. It'd be a good topic for a maintainer summit one day.
Re: [PATCH 2/2] Compiler Attributes: auxdisplay: panel: use __fallthrough
On Sun, Oct 21, 2018 at 10:14 AM, Miguel Ojeda
wrote:
> Let gcc know these cases are meant to fall through to the next label
> by annotating them with the new __fallthrough statement attribute;
> and remove the comment since it conveys the same information
> (which was also parsed by gcc to suppress the warning).
>
> Signed-off-by: Miguel Ojeda
> ---
> drivers/auxdisplay/panel.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/auxdisplay/panel.c b/drivers/auxdisplay/panel.c
> index 21b9b2f2470a..0755034e49ba 100644
> --- a/drivers/auxdisplay/panel.c
> +++ b/drivers/auxdisplay/panel.c
> @@ -1367,7 +1367,7 @@ static void panel_process_inputs(void)
> break;
> input->rise_timer = 0;
> input->state = INPUT_ST_RISING;
> - /* fall through */
> + __fallthrough;
> case INPUT_ST_RISING:
> if ((phys_curr & input->mask) != input->value) {
> input->state = INPUT_ST_LOW;
> @@ -1380,11 +1380,11 @@ static void panel_process_inputs(void)
> }
> input->high_timer = 0;
> input->state = INPUT_ST_HIGH;
> - /* fall through */
> + __fallthrough;
> case INPUT_ST_HIGH:
> if (input_state_high(input))
> break;
> - /* fall through */
> + __fallthrough;
> case INPUT_ST_FALLING:
> input_state_falling(input);
> }
> --
> 2.17.1
>
I would prefer we continue to use the comment style until we've got
confirmed support for (at least) Clang, Coverity, CPPcheck, smatch,
and eclipse.
-Kees
--
Kees Cook
Pixel Security
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 1:24 PM Miguel Ojeda wrote: > > On Mon, Oct 22, 2018 at 12:53 PM Kees Cook wrote: > > > > On Mon, Oct 22, 2018 at 2:41 AM, Miguel Ojeda > > wrote: > > > > > * clang does *not* support the attribute in C. > > > > Well that's not good. :) > > I will see with clang if they plan to add it. > So I have looked in the clang sources and I have seen that clang is already preparing for C2x: since clang >= 6 we can actually enable the C++-like attributes with "-fdouble-square-bracket-attributes" in C mode, which in turn makes the warning work in C mode and can be suppressed with [[fallthrough]]. This would give us the warning also in clang, which would be a win vs. the comments. Nick? However, I don't see a way to enable [[fallthrough]] in C mode for gcc >= 7.1 -- from a quick look, the C parser does not know about [[]] attributes, so I don't think we can use [[fallthrough]] for both compilers (yet). Cheers, Miguel
[PATCH v9 9/9] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander
Document the Cadence I3C gpio expander bindings.
Signed-off-by: Boris Brezillon
Reviewed-by: Rob Herring
Reviewed-by: Linus Walleij
---
Changes in v9:
- Add Linus R-b
Changes in v8:
- None
Changes in v7:
- None
Changes in v6:
- None
Changes in v5:
- Add Rob's R-b
Changes in v4:
- Use GPIO_ and IRQ_TYPE_ macros instead of raw numbers
- Fix the unit-address in the example
---
.../bindings/gpio/gpio-cdns-i3c.txt | 39 +++
1 file changed, 39 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
diff --git a/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
new file mode 100644
index ..d0155a9cea79
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
@@ -0,0 +1,39 @@
+* Cadence I3C GPIO expander
+
+The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
+This GPIOs can be configured in output or input mode and if they are in input
+mode they can generate IBIs (In Band Interrupts).
+
+Required properties for GPIO node:
+- reg : 3 cells encoding the I3C static address (none in our case) and the I3C
+ Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt for
+ more details.
+ Should be <0x0 0x392 0x0>.
+- gpio-controller : Marks the device node as a gpio controller.
+- #gpio-cells : Should be two. The first cell is the pin number and
+ the second cell is used to specify the gpio polarity (GPIO_ACTIVE_HIGH or
+ GPIO_ACTIVE_LOW)
+- interrupt-controller: Marks the device node as an interrupt controller.
+- #interrupt-cells : Should be 2. The first cell is the GPIO number.
+ The second cell is used to specify trigger type and level flags.
+ The following trigger types are accepted (see
+ for their definition):
+ IRQ_TYPE_EDGE_RISING
+ IRQ_TYPE_EDGE_FALLING
+ IRQ_TYPE_EDGE_BOTH
+ IRQ_TYPE_LEVEL_HIGH
+ IRQ_TYPE_LEVEL_LOW
+
+Example:
+
+ i3c-master@xxx {
+ ...
+ i3c_gpio_expander: gpio@0,392 {
+ reg = <0 0x392 0x0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ ...
+ };
--
2.17.1
[PATCH v9 5/9] MAINTAINERS: Add myself as the I3C subsystem maintainer
Create an entry for the I3C subsystem and mark it as maintained by me. There's no official git repository, patchwork instance, mailing list or website yet, but this will be added after the subsystem has been accepted. Signed-off-by: Boris Brezillon --- Changes in v9: - None Changes in v8: - None Changes in v7: - Add a ML (not created yet) and a git repo Changes in v5: - Add the sysfs ABI file --- MAINTAINERS | 12 1 file changed, 12 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a5b256b25905..293c863b6f9f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6918,6 +6918,18 @@ L: linux-...@vger.kernel.org S: Maintained F: drivers/i2c/i2c-stub.c +I3C SUBSYSTEM +M: Boris Brezillon +L: linux-...@vger.kernel.org +T: git git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git +S: Maintained +F: Documentation/ABI/testing/sysfs-bus-i3c +F: Documentation/devicetree/bindings/i3c/ +F: Documentation/driver-api/i3c +F: drivers/i3c/ +F: include/linux/i3c/ +F: include/dt-bindings/i3c/ + IA64 (Itanium) PLATFORM M: Tony Luck M: Fenghua Yu -- 2.17.1
[PATCH v9 7/9] dt-bindings: i3c: Document Cadence I3C master bindings
Document Cadence I3C master DT bindings.
Signed-off-by: Boris Brezillon
Reviewed-by: Rob Herring
Reviewed-by: Arnd Bergmann
---
Changes in v9:
- Add Arnd's R-b
Changes in v8:
- None
Changes in v7:
- None
Changes in v6:
- None
Changes in v5:
- Add Rob's R-b
Changes in v4:
- Fix example to match the new representation
---
.../bindings/i3c/cdns,i3c-master.txt | 44 +++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
diff --git a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
new file mode 100644
index ..0e2b8b8770bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
@@ -0,0 +1,44 @@
+Bindings for cadence I3C master block
+=
+
+Required properties:
+
+- compatible: shall be "cdns,i3c-master"
+- clocks: shall reference the pclk and sysclk
+- clock-names: shall contain "pclk" and "sysclk"
+- interrupts: the interrupt line connected to this I3C master
+- reg: I3C master registers
+
+Mandatory properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- #address-cells: shall be set to 1
+- #size-cells: shall be set to 0
+
+Optional properties defined by the generic binding (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details):
+
+- i2c-scl-hz
+- i3c-scl-hz
+
+I3C device connected on the bus follow the generic description (see
+Documentation/devicetree/bindings/i3c/i3c.txt for more details).
+
+Example:
+
+ i3c-master@0d04 {
+ compatible = "cdns,i3c-master";
+ clocks = <&coreclock>, <&i3csysclock>;
+ clock-names = "pclk", "sysclk";
+ interrupts = <3 0>;
+ reg = <0x0d04 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ i2c-scl-hz = <10>;
+
+ nunchuk: nunchuk@52 {
+ compatible = "nintendo,nunchuk";
+ reg = <0x52 0x8010 0>;
+ };
+ };
+
--
2.17.1
[PATCH v9 4/9] dt-bindings: i3c: Document core bindings
A new I3C subsystem has been added and a generic description has been
created to represent the I3C bus and the devices connected on it.
Document this generic representation.
Cc: Rob Herring
Signed-off-by: Boris Brezillon
---
Rob,
Can you have a look at this v9. I know you previously acked this
binding but I changed a bit the way we declare I2C devices and thus
had to drop your Ack.
The IS_I2C_DEV bit no long exists, and we instead consider that any
device with the 2nd reg entry set to 0 is an I2C device, which is
just fine because this entry is supposed to contain the upper part of
the Provisional ID which is basically the MIPI manufacturer ID,
and 0 is not a valid manufacturer ID.
By doing that we make reg definition more straightforward and can
drop the ugly {I2C,I3C}_DEV() macros.
Regards,
Boris
Changes in v9:
- Rework the way we encode the I2C vs I3C device in the reg property
so that we don't need the funky macros to define and I3C or I2C dev
- Drop Rob's Reviewed-by
Changes in v8:
- None
Changes in v7:
- None
Changes in v6:
- None
Changes in v5:
- Add Rob's R-b
Changes in v4:
- Clarify the fact that static address == I3C address and dynamic
address == I3C address
- Use i2c-scl-hz in the example
Changes in v3:
- Rename {i2c,i3c}-scl-frequency DT prop into {i2c,i3c}-scl-hz
- Rework the way we expose the provisional ID and LVR information
- Rename dynamic-address into assigned-address
- Enforce the I3C master node name
Changes in v2:
- Define how to describe I3C devices in the DT and when it should be
used. Note that the parsing of I3C devices is not yet implemented in
the framework. Will be added when someone really needs it.
---
Documentation/devicetree/bindings/i3c/i3c.txt | 138 ++
1 file changed, 138 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
diff --git a/Documentation/devicetree/bindings/i3c/i3c.txt
b/Documentation/devicetree/bindings/i3c/i3c.txt
new file mode 100644
index ..7021468e0205
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/i3c.txt
@@ -0,0 +1,138 @@
+Generic device tree bindings for I3C busses
+===
+
+This document describes generic bindings that should be used to describe I3C
+busses in a device tree.
+
+Required properties
+---
+
+- #address-cells - should be <3>. Read more about addresses below.
+- #size-cells - should be <0>.
+- compatible - name of the I3C master controller driving the I3C bus
+
+For other required properties e.g. to describe register sets,
+clocks, etc. check the binding documentation of the specific driver.
+The node describing an I3C bus should be named i3c-master.
+
+Optional properties
+---
+
+These properties may not be supported by all I3C master drivers. Each I3C
+master bindings should specify which of them are supported.
+
+- i3c-scl-hz: frequency of the SCL signal used for I3C transfers.
+ When undefined the core sets it to 12.5MHz.
+
+- i2c-scl-hz: frequency of the SCL signal used for I2C transfers.
+ When undefined, the core looks at LVR (Legacy Virtual Register)
+ values of I2C devices described in the device tree to determine
+ the maximum I2C frequency.
+
+I2C devices
+===
+
+Each I2C device connected to the bus should be described in a subnode. All
+properties described in Documentation/devicetree/bindings/i2c/i2c.txt are
+valid here, but several new properties have been added.
+
+New constraint on existing properties:
+--
+- reg: contains 3 cells
+ + first cell : still encoding the I2C address
+
+ + second cell: shall be 0
+
+ + third cell: shall encode the I3C LVR (Legacy Virtual Register)
+ bit[31:8]: unused/ignored
+ bit[7:5]: I2C device index. Possible values
+ * 0: I2C device has a 50 ns spike filter
+ * 1: I2C device does not have a 50 ns spike filter but supports high
+frequency on SCL
+ * 2: I2C device does not have a 50 ns spike filter and is not tolerant
+to high frequencies
+ * 3-7: reserved
+
+ bit[4]: tell whether the device operates in FM (Fast Mode) or FM+ mode
+ * 0: FM+ mode
+ * 1: FM mode
+
+ bit[3:0]: device type
+ * 0-15: reserved
+
+I3C devices
+===
+
+All I3C devices are supposed to support DAA (Dynamic Address Assignment), and
+are thus discoverable. So, by default, I3C devices do not have to be described
+in the device tree.
+This being said, one might want to attach extra resources to these devices,
+and those resources may have to be described in the device tree, which in turn
+means we have to describe I3C devices.
+
+Another use case for describing an I3C device in the device tree is when this
+I3C device has a static I2C address and we want to assign it a specific I3C
+dynamic address before the DAA takes place (so that other devices on the bus
+can
[PATCH v9 3/9] i3c: Add sysfs ABI spec
Document sysfs files/directories/symlinks exposed by the I3C subsystem. Signed-off-by: Boris Brezillon Reviewed-by: Arnd Bergmann --- Changes in v9: - Add Arnd's R-b Changes in v8: - None Changes in v7: - Bump KernelVersion to 4.20 - Add new entries under i3c- now that the master controller is no longer represented as a device under this directory but implicitly represented by the bus object Changes in v6: - none Changes in v5: - Fix the kernel version - Rename address into dynamic_address to match changes done in the code Changes in v4: - none Changes in v3: - none Changes in v2: - new patch --- Documentation/ABI/testing/sysfs-bus-i3c | 146 1 file changed, 146 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-i3c diff --git a/Documentation/ABI/testing/sysfs-bus-i3c b/Documentation/ABI/testing/sysfs-bus-i3c new file mode 100644 index ..d7c7485b0c17 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-i3c @@ -0,0 +1,146 @@ +What: /sys/bus/i3c/devices/i3c- +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + An I3C bus. This directory will contain one sub-directory per + I3C device present on the bus. + +What: /sys/bus/i3c/devices/i3c-/current_master +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + Expose the master that owns the bus (-) at + the time this file is read. Note that bus ownership can change + overtime, so there's no guarantee that when the read() call + returns, the value returned is still valid. + +What: /sys/bus/i3c/devices/i3c-/mode +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + I3C bus mode. Can be "pure", "mixed-fast" or "mixed-slow". See + the I3C specification for a detailed description of what each + of these modes implies. + +What: /sys/bus/i3c/devices/i3c-/i3c_scl_frequency +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + The frequency (expressed in Hz) of the SCL signal when + operating in I3C SDR mode. + +What: /sys/bus/i3c/devices/i3c-/i2c_scl_frequency +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + The frequency (expressed in Hz) of the SCL signal when + operating in I2C mode. + +What: /sys/bus/i3c/devices/i3c-/dynamic_address +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + Dynamic address assigned to the master controller. This + address may change if the bus is re-initialized. + +What: /sys/bus/i3c/devices/i3c-/bcr +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + BCR stands for Bus Characteristics Register and express the + device capabilities in term of speed, maximum read/write + length, etc. See the I3C specification for more details. + This entry describes the BCR of the master controller driving + the bus. + +What: /sys/bus/i3c/devices/i3c-/dcr +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + DCR stands for Device Characteristics Register and express the + device capabilities in term of exposed features. See the I3C + specification for more details. + This entry describes the DCR of the master controller driving + the bus. + +What: /sys/bus/i3c/devices/i3c-/pid +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + PID stands for Provisional ID and is used to uniquely identify + a device on a bus. This PID contains information about the + vendor, the part and an instance ID so that several devices of + the same type can be connected on the same bus. + See the I3C specification for more details. + This entry describes the PID of the master controller driving + the bus. + +What: /sys/bus/i3c/devices/i3c-/hdrcap +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + Expose the HDR (High Data Rate) capabilities of a device. + Returns a list of supported HDR mode, each element is separated + by space. Modes can be "hdr-ddr", "hdr-tsp" and "hdr-tsl". + See the I3C specification for more details about these HDR + modes. + This entry describes the HDRCAP of the master controller + driving the bus. + +What: /sys/bus/i3c/devices/i3c-/- +KernelVersion: 4.20 +Contact: linux-...@vger.kernel.org +Description: + An I3C device present on I3C bus identified by . Note
[PATCH v9 8/9] gpio: Add a driver for Cadence I3C GPIO expander
Add a driver for Cadence I3C GPIO expander.
Signed-off-by: Boris Brezillon
Acked-by: Linus Walleij
---
Changes in v9:
- None
Changes in v8:
- None
Changes in v7:
- None
Changes in v6:
- Use kmalloc_array() instead of kmalloc(N * sizeof(X))
- Add Linus' ack
Changes in v5:
- Use the !! operator to return 0 or 1 in cdns_i3c_gpio_get_direction()
- Use a scratch buffer to make sure the buffers passed to
i3c_device_do_priv_xfers() are DMA-able
- Fix errors reported by checkpatch
Changes in v4:
- none
Changes in v3:
- new
---
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile| 1 +
drivers/gpio/gpio-cdns-i3c.c | 411 +++
3 files changed, 423 insertions(+)
create mode 100644 drivers/gpio/gpio-cdns-i3c.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4f52c3a8ec99..a941601bf448 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -908,6 +908,17 @@ config GPIO_TS4900
endmenu
+menu "I3C GPIO expanders"
+ depends on I3C
+
+config GPIO_CDNS_I3C
+ tristate "Cadence I3C GPIO expander"
+ select GPIOLIB_IRQCHIP
+ help
+ Say yes here to enabled the driver for Cadence I3C GPIO expander.
+
+endmenu
+
menu "MFD GPIO expanders"
config GPIO_ADP5520
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c256aff66a65..12fa0ace618e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o
obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o
obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o
obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o
+obj-$(CONFIG_GPIO_CDNS_I3C)+= gpio-cdns-i3c.o
obj-$(CONFIG_GPIO_CLPS711X)+= gpio-clps711x.o
obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o
obj-$(CONFIG_GPIO_CRYSTAL_COVE)+= gpio-crystalcove.o
diff --git a/drivers/gpio/gpio-cdns-i3c.c b/drivers/gpio/gpio-cdns-i3c.c
new file mode 100644
index ..37c9d57a2fbb
--- /dev/null
+++ b/drivers/gpio/gpio-cdns-i3c.c
@@ -0,0 +1,411 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Cadence Design Systems Inc.
+ *
+ * Author: Boris Brezillon
+ */
+
+#include
+#include
+#include
+
+#define OVR0x0
+#define IVR0x1
+#define DIR_MODE 0x2
+#define IMR0x3
+#define ISR0x4
+#define ITR(x) (0x5 + (x))
+
+struct cdns_i3c_gpio {
+ struct gpio_chip gpioc;
+ struct irq_chip irqc;
+ struct i3c_device *i3cdev;
+ struct mutex irq_lock;
+ u8 dir;
+ u8 ovr;
+ u8 imr;
+ u8 itr[3];
+};
+
+static struct cdns_i3c_gpio *gpioc_to_cdns_gpioc(struct gpio_chip *gpioc)
+{
+ return container_of(gpioc, struct cdns_i3c_gpio, gpioc);
+}
+
+static int cdns_i3c_gpio_read_reg(struct cdns_i3c_gpio *gpioc, u8 reg,
+ u8 *val)
+{
+ struct i3c_priv_xfer xfers[2] = { };
+ u8 *scratchbuf;
+ int ret;
+
+ /*
+* i3c_device_do_priv_xfers() mandates that buffers passed in xfers be
+* DMA-able. This prevents us from using reg and val directly since
+* reg is on the stack, and val might be too.
+* Allocate a temporary buffer with kmalloc() to solve the problem.
+*/
+ scratchbuf = kmalloc(sizeof(*scratchbuf), GFP_KERNEL);
+ if (!scratchbuf)
+ return -ENOMEM;
+
+ scratchbuf[0] = reg;
+ xfers[0].data.out = scratchbuf;
+ xfers[0].len = 1;
+ xfers[1].data.in = scratchbuf;
+ xfers[1].len = 1;
+ xfers[1].rnw = true;
+
+ ret = i3c_device_do_priv_xfers(gpioc->i3cdev, xfers,
+ ARRAY_SIZE(xfers));
+ if (!ret)
+ *val = *scratchbuf;
+
+ kfree(scratchbuf);
+
+ return ret;
+}
+
+static int cdns_i3c_gpio_write_reg(struct cdns_i3c_gpio *gpioc, u8 reg,
+ u8 val)
+{
+ struct i3c_priv_xfer xfers[2] = { };
+ u8 *scratchbuf;
+ int ret;
+
+ /*
+* i3c_device_do_priv_xfers() mandates that buffers passed in xfers be
+* DMA-able. This prevents us from using reg and val directly since
+* reg is on the stack, and val might be too.
+* Allocate a temporary buffer with kmalloc() to solve the problem.
+*/
+ scratchbuf = kmalloc_array(2, sizeof(*scratchbuf), GFP_KERNEL);
+ if (!scratchbuf)
+ return -ENOMEM;
+
+ scratchbuf[0] = reg;
+ scratchbuf[1] = val;
+ xfers[0].data.out = scratchbuf;
+ xfers[0].len = 1;
+ xfers[1].data.out = scratchbuf + 1;
+ xfers[1].len = 1;
+
+ ret = i3c_device_do_priv_xfers(gpioc->i3cdev, xfers,
+ ARRAY_SIZE(xfers));
+
+ kfree(scratchbuf);
+
+ return ret;
+}
+
+static int cdns_i3c_gpio_get_direction(struct gpio_chip *g,
+ unsigned int offset)
+{
+ struct cdns_i3c_gpi
[PATCH v9 2/9] docs: driver-api: Add I3C documentation
Add the I3C documentation describing the protocol, the master driver API and the device driver API. Signed-off-by: Boris Brezillon Reviewed-by: Randy Dunlap Reviewed-by: Arnd Bergmann --- Changes in v9: - Add Arnd's R-b Changes in v8: - None Changes in v7: - None Changes in v6: - Fix typos reported by Randy - Add Randy's R-b Changes in v5: - Remove useless conf.py file - Add SPDX headers Changes in v2: - Moved out of patch "i3c: Add core I3C infrastructure" - Add link to the I3C spec - Move rst files in Documentation/driver-api/i3c/ --- .../driver-api/i3c/device-driver-api.rst | 9 + Documentation/driver-api/i3c/index.rst| 11 + .../driver-api/i3c/master-driver-api.rst | 10 + Documentation/driver-api/i3c/protocol.rst | 203 ++ Documentation/driver-api/index.rst| 1 + 5 files changed, 234 insertions(+) create mode 100644 Documentation/driver-api/i3c/device-driver-api.rst create mode 100644 Documentation/driver-api/i3c/index.rst create mode 100644 Documentation/driver-api/i3c/master-driver-api.rst create mode 100644 Documentation/driver-api/i3c/protocol.rst diff --git a/Documentation/driver-api/i3c/device-driver-api.rst b/Documentation/driver-api/i3c/device-driver-api.rst new file mode 100644 index ..85bc3381cd3e --- /dev/null +++ b/Documentation/driver-api/i3c/device-driver-api.rst @@ -0,0 +1,9 @@ +.. SPDX-License-Identifier: GPL-2.0 + += +I3C device driver API += + +.. kernel-doc:: include/linux/i3c/device.h + +.. kernel-doc:: drivers/i3c/device.c diff --git a/Documentation/driver-api/i3c/index.rst b/Documentation/driver-api/i3c/index.rst new file mode 100644 index ..783d6dad054b --- /dev/null +++ b/Documentation/driver-api/i3c/index.rst @@ -0,0 +1,11 @@ +.. SPDX-License-Identifier: GPL-2.0 + += +I3C subsystem += + +.. toctree:: + + protocol + device-driver-api + master-driver-api diff --git a/Documentation/driver-api/i3c/master-driver-api.rst b/Documentation/driver-api/i3c/master-driver-api.rst new file mode 100644 index ..bb19264aa239 --- /dev/null +++ b/Documentation/driver-api/i3c/master-driver-api.rst @@ -0,0 +1,10 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +I3C master controller driver API + + +.. kernel-doc:: drivers/i3c/master.c + +.. kernel-doc:: include/linux/i3c/master.h + diff --git a/Documentation/driver-api/i3c/protocol.rst b/Documentation/driver-api/i3c/protocol.rst new file mode 100644 index ..dae3b6d32c6b --- /dev/null +++ b/Documentation/driver-api/i3c/protocol.rst @@ -0,0 +1,203 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +I3C protocol + + +Disclaimer +== + +This chapter will focus on aspects that matter to software developers. For +everything hardware related (like how things are transmitted on the bus, how +collisions are prevented, ...) please have a look at the I3C specification. + +This document is just a brief introduction to the I3C protocol and the concepts +it brings to the table. If you need more information, please refer to the MIPI +I3C specification (can be downloaded here +http://resources.mipi.org/mipi-i3c-v1-download). + +Introduction + + +The I3C (pronounced 'eye-three-see') is a MIPI standardized protocol designed +to overcome I2C limitations (limited speed, external signals needed for +interrupts, no automatic detection of the devices connected to the bus, ...) +while remaining power-efficient. + +I3C Bus +=== + +An I3C bus is made of several I3C devices and possibly some I2C devices as +well, but let's focus on I3C devices for now. + +An I3C device on the I3C bus can have one of the following roles: + +* Master: the device is driving the bus. It's the one in charge of initiating + transactions or deciding who is allowed to talk on the bus (slave generated + events are possible in I3C, see below). +* Slave: the device acts as a slave, and is not able to send frames to another + slave on the bus. The device can still send events to the master on + its own initiative if the master allowed it. + +I3C is a multi-master protocol, so there might be several masters on a bus, +though only one device can act as a master at a given time. In order to gain +bus ownership, a master has to follow a specific procedure. + +Each device on the I3C bus has to be assigned a dynamic address to be able to +communicate. Until this is done, the device should only respond to a limited +set of commands. If it has a static address (also called legacy I2C address), +the device can reply to I2C transfers. + +In addition to these per-device addresses, the protocol defines a broadcast +address in order to address all devices on the bus. + +Once a dynamic address has been assigned to a device, this address will be used +for any direct communication with the device. Note that even after being +as
[PATCH v9 0/9] Add the I3C subsystem
Hi Greg,
I think we've reached a point where we can eventually consider the I3C
framework for inclusion in 4.20 (5.0?). Indeed, v8 has received
a R-b from Arnd, and I addressed the concerned raised by Joe on the
I3C/I2C_DEV() macros. I hope to get an Ack from Rob on the adjusted
DT bindings anytime soon.
The only remaining concern raised by Arnd is the fact that both hosts
and slaves share the same bus type and are differentiated thanks to
their device_type, which IMHO is fine since this is what other
subsystems do (plus I don't see other solutions to have both I3C
devices and I3C buses represented under /sys/bus/i3c/).
I'd really like to get this series merged in 4.20 so that other
contributors can work on top of that to add
1/ new host drivers
2/ support for advanced features
3/ new device drivers
So, unless you have strong reasons to reject this request, and,
assuming I get Rob's ack soon enough, I plan to send a PR for this
framework next week.
Here is the usual description copy&pasted from the previous versions:
This patch series is adding a new subsystem to support I3C devices.
This is just adding support for basic features. Extra features will
be added afterwards.
There are a few design choices that are worth mentioning because they
impact the way I3C device drivers can interact with their devices:
- all functions used to send I3C/I2C frames must be called in
non-atomic context. Mainly done this way to ease implementation, but
this is still open to discussion. Please let me know if you think it's
worth considering an asynchronous model here
- the I3C bus and I3C master controller are now tightly coupled even
though they're still allocated separately. There's now a 1:1
relationship between these objects, and the I3C master is no longer
represented under the I3C bus object.
Arnd, let me know if you had something different in mind, and I'll
rework the implementation accordingly.
- I2C backward compatibility has been designed to be transparent to I2C
drivers and the I2C subsystem. The I3C master just registers an I2C
adapter which creates a new I2C bus. I'd say that, from a
representation PoV it's not ideal because what should appear as a
single I3C bus exposing I3C and I2C devices here appears as 2
different busses connected to each other through the parenting (the
I3C master is the parent of the I2C and I3C busses).
On the other hand, I don't see a better solution if we want something
that is not invasive.
Missing features (will be added in separate patch series after initial
support has been accepted/merged):
- support for HDR modes (has been removed because of lack of real users)
- no support for multi-master and the associated concepts (mastership
handover, support for secondary masters, ...)
- I2C devices can only be described using DT because this is the only
use case I have. However, the framework can easily be extended with
ACPI and board info support
- I3C slave framework. This has been completely omitted, but shouldn't
have a huge impact on the I3C framework because I3C slaves don't see
the whole bus, it's only about handling master requests and generating
IBIs. Some of the struct, constant and enum definitions could be
shared, but most of the I3C slave framework logic will be different
Note that this patchset is available on the linux-i3c repo[1].
Main change between v8 and v9:
- The DT bindings have been simplified to get rid of the I3C/I3C_DEV()
macros
Main change between v7 and v8:
- The bus object is now embedded in the master_controller object (as
suggested by Arnd)
Main changes between v6 and v7:
- I3C bus/master representations have been reworked to match what other
subsystems are doing (master implicitly represented by the bus
object)
- I3C dev registration has been fixed
- I3C bus mode selection has been fixed
- Calls to readsl/writesl() in the Cadence I3C master driver have been
fixed
Main changes between v5 and v6:
- Introduce {i3c,i2c}_dev_desc structures to better match how I3C
master controllers (reservation of one HW slot for each device
attached to the bus). With this solution, the resource migration
that happens when a device lose its dynamic address and is
re-assigned a different address is simplified on the driver side,
because most of it is now handled in the core (reserve a new dev
slot, reserve IBI resources and free all resources attached to the
old slot)
- Add I3C error codes (M0 to M2) so that the core and device drivers
can have fine grained information on what caused an EIO error.
Only minor things happened between v3 and v5 (you can go check the
changelog in each patch for more details).
Main changes between v2 and v3 are:
- Reworked the DT bindings as suggested by Rob
- Reworked the bus initialization step as suggested by Vitor
- Added a driver for an I3C GPIO expander
Main changes between the initial RFC and this v2 are:
- Add a generic infrastructure to support IBIs. It'
[PATCH v9 6/9] i3c: master: Add driver for Cadence IP
Add a driver for Cadence I3C master IP.
Signed-off-by: Boris Brezillon
Reviewed-by: Arnd Bergmann
---
Changes in v9:
- Change the 'depends on' rule (suggested by Arnd)
- Use {read,write}sl() instead of __raw_{readl,writem}() (suggested by
Arnd)
- Add Arnd's R-b
Changes in v8:
- Adjust code to match changes done in the core (bus embedded in master)
Changes in v7:
- Fix readsl/writesl() usage
- Add a depends on ARM || ARM64 || XTENSA to forbid selection of this
driver on platforms that are not implementing readsl/writesl
Changes in v6:
- Rework the attach/detach logic to match the new way of doing things.
- Use kcalloc() where appropriate
- Simplfy access to the RX/TX/IBI_DATA FIFOs
Changes in v5:
- Drop unused len var in cdns_i3c_master_handle_ibi()
- Get IBIR and CMDR depth from CONF_STATUS0
- s/2017/2018/ in copyright header
- Fix coding style issues
Changes in v4:
- Fix potential unsigned integer underflow
- Add missing static specific on IBI related functions
Changes in v3:
- Adjust to match I3C framework changes
- Implement support the CMD RESPONSE QUEUE and IBI QUEUE added in the
latest revision of Cadence master IP
- Remove support for HDR modes
Changes in v2:
- Add basic IBI support. Note that the IP is not really reliable with
regards to IBI because you can't extract IBI payloads as soon as you
have more than one IBI waiting in the HW queue. This is something
that will hopefully be addressed in future revisions of this IP
- Add a simple xfer queueing mechanism to optimize message queuing.
- Fix a few bugs
- Add support for Hot Join
---
drivers/i3c/master/Kconfig |6 +
drivers/i3c/master/Makefile |1 +
drivers/i3c/master/i3c-master-cdns.c | 1671 ++
3 files changed, 1678 insertions(+)
create mode 100644 drivers/i3c/master/i3c-master-cdns.c
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index e69de29bb2d1..959b837509fe 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -0,0 +1,6 @@
+config CDNS_I3C_MASTER
+ tristate "Cadence I3C master driver"
+ depends on I3C
+ depends on !(ALPHA || PARISC)
+ help
+ Enable this driver if you want to support Cadence I3C master block.
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index e69de29bb2d1..4c4304aa9534 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_CDNS_I3C_MASTER) += i3c-master-cdns.o
diff --git a/drivers/i3c/master/i3c-master-cdns.c
b/drivers/i3c/master/i3c-master-cdns.c
new file mode 100644
index ..4f5994294eb0
--- /dev/null
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -0,0 +1,1671 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Cadence Design Systems Inc.
+ *
+ * Author: Boris Brezillon
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEV_ID 0x0
+#define DEV_ID_I3C_MASTER 0x5034
+
+#define CONF_STATUS0 0x4
+#define CONF_STATUS0_CMDR_DEPTH(x) (4 << (((x) & GENMASK(31, 29)) >> 29))
+#define CONF_STATUS0_ECC_CHK BIT(28)
+#define CONF_STATUS0_INTEG_CHK BIT(27)
+#define CONF_STATUS0_CSR_DAP_CHK BIT(26)
+#define CONF_STATUS0_TRANS_TOUT_CHKBIT(25)
+#define CONF_STATUS0_PROT_FAULTS_CHK BIT(24)
+#define CONF_STATUS0_GPO_NUM(x)(((x) & GENMASK(23, 16)) >> 16)
+#define CONF_STATUS0_GPI_NUM(x)(((x) & GENMASK(15, 8)) >> 8)
+#define CONF_STATUS0_IBIR_DEPTH(x) (4 << (((x) & GENMASK(7, 6)) >> 7))
+#define CONF_STATUS0_SUPPORTS_DDR BIT(5)
+#define CONF_STATUS0_SEC_MASTERBIT(4)
+#define CONF_STATUS0_DEVS_NUM(x) ((x) & GENMASK(3, 0))
+
+#define CONF_STATUS1 0x8
+#define CONF_STATUS1_IBI_HW_RES(x) x) & GENMASK(31, 28)) >> 28) + 1)
+#define CONF_STATUS1_CMD_DEPTH(x) (4 << (((x) & GENMASK(27, 26)) >> 26))
+#define CONF_STATUS1_SLVDDR_RX_DEPTH(x)(8 << (((x) & GENMASK(25, 21))
>> 21))
+#define CONF_STATUS1_SLVDDR_TX_DEPTH(x)(8 << (((x) & GENMASK(20, 16))
>> 16))
+#define CONF_STATUS1_IBI_DEPTH(x) (2 << (((x) & GENMASK(12, 10)) >> 10))
+#define CONF_STATUS1_RX_DEPTH(x) (8 << (((x) & GENMASK(9, 5)) >> 5))
+#define CONF_STATUS1_TX_DEPTH(x) (8 << ((x) & GENMASK(4, 0)))
+
+#define REV_ID 0xc
+#define REV_ID_VID(id) (((id) & GENMASK(31, 20)) >> 20)
+#define REV_ID_PID(id) (((id) & GENMASK(19, 8)) >> 8)
+#define REV_ID_REV_MAJOR(id) (((id) & GENMASK(7, 4)) >> 4)
+#define REV_ID_REV_MINOR(id) ((id) & GENMASK(3, 0))
+
+#define CTRL 0x10
+#define CTRL_DEV_ENBIT(31)
+#define CTRL_HALT_EN BIT(30)
+#define CTRL_MC
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:07 PM Luc Van Oostenryck wrote: > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > > > While comment parsing is a good idea to deal with old codebases > > that used such a comment as documentation for humans, the best > > solution is to use the attribute: > > > > * It is a "real" part of the AST (=> better for tooling). > > This will create a problem for the recent versions of sparse which > support __has_attribute() because sparse falsely pretends to support > this attribute while, to play nice with -Wdeclaration-after-statement, > it needs some adaptation to the parsing (it's actually seen not as a > statement but as a declaration). I'll see to fix it this evening. No rush Luc! (I have sent the PR to Linus without this two patches for the moment). And thanks a lot for having being so quick to improve sparse to support all these series! Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > While comment parsing is a good idea to deal with old codebases > that used such a comment as documentation for humans, the best > solution is to use the attribute: > > * It is a "real" part of the AST (=> better for tooling). This will create a problem for the recent versions of sparse which support __has_attribute() because sparse falsely pretends to support this attribute while, to play nice with -Wdeclaration-after-statement, it needs some adaptation to the parsing (it's actually seen not as a statement but as a declaration). I'll see to fix it this evening. Regards, -- Luc
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 12:53 PM Kees Cook wrote: > > On Mon, Oct 22, 2018 at 2:41 AM, Miguel Ojeda > wrote: > > Please take a look at the rationale (also more details at the linked > > thread): > > > > * gcc 7.1 added -Wimplicit-fallthrough at the same time as the > > attribute and the comment parsing. > > Ah, perfect. I missed this. :) No problem! I know the commit message is a bit too long, so I understand :) > > > * clang does *not* support the attribute in C. > > Well that's not good. :) I will see with clang if they plan to add it. (By the way, if the "*not*" sounded rude, sorry; I wanted to emphasize it is surprising that it doesn't -- I also assumed the opposite until I checked it). Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 12:54 PM Dan Carpenter wrote: > > Doing both is super ugly. Let's just do comments until Eclipse gets > updated. > > I had wanted to move to the attribute because that would simplify things > in Smatch but it's not a huge deal to delay for another year. I can re-send them later on, no problem. On the other hand, doing the changes will push tools to get updated sooner ;-) If tools were doing something as fancy as comment parsing for diagnostics, they should have been updated with the attribute support (either gcc's or C++17's) -- it has been more than a year now since gcc 7.1 and the C++17 final draft. (Note that this does not apply for things like clang, since they weren't doing comment parsing to begin with.) Cheers, Miguel
[GIT PULL] Compiler Attributes for v4.20-rc1
Hi Linus, Here it is the Compiler Attributes series/tree, which tries to disentangle the include/linux/compiler*.h headers and bring them up to date. The patches have been in linux-next for a while, *except* the last two from Nick which came a bit later. Since AFAIU there will be no linux-next this week, I included them here; but let me know if you prefer to take them out. You may see merge conflicts from a few other trees, from what we have seen in linux-next. I am not sure if you followed this, so please let me know if there is anything you don't agree with. And welcome back! Cheers, Miguel The following changes since commit 17b57b1883c1285f3d0dc2266e8f79286a7bef38: Linux 4.19-rc6 (2018-09-30 07:15:35 -0700) are available in the Git repository at: https://github.com/ojeda/linux.git tags/compiler-attributes-for-linus-4.20-rc1 for you to fetch changes up to 1ff2fea5e30ca15752777441ecb64a169fe22e9e: compiler-gcc: remove comment about gcc 4.5 from unreachable() (2018-10-19 08:47:30 +0200) The Compiler Attributes series This is an effort to disentangle the include/linux/compiler*.h headers and bring them up to date. The main idea behind the series is to use feature checking macros (i.e. __has_attribute) instead of compiler version checks (e.g. GCC_VERSION), which are compiler-agnostic (so they can be shared, reducing the size of compiler-specific headers) and version-agnostic. Other related improvements have been performed in the headers as well, which on top of the use of __has_attribute it has amounted to a significant simplification of these headers (e.g. GCC_VERSION is now only guarding a few non-attribute macros). This series should also help the efforts to support compiling the kernel with clang and icc. A fair amount of documentation and comments have also been added, clarified or removed; and the headers are now more readable, which should help kernel developers in general. The series was triggered due to the move to gcc >= 4.6. In turn, this series has also triggered Sparse to gain the ability to recognize __has_attribute on its own. Finally, the __nonstring variable attribute series has been also applied on top; plus two related patches from Nick Desaulniers for unreachable() that came a bit afterwards. Miguel Ojeda (15): Compiler Attributes: remove unused attributes Compiler Attributes: always use the extra-underscores syntax Compiler Attributes: remove unneeded tests Compiler Attributes: homogenize __must_be_array Compiler Attributes: remove unneeded sparse (__CHECKER__) tests Compiler Attributes: add missing SPDX ID in compiler_types.h Compiler Attributes: use feature checks instead of version checks Compiler Attributes: KENTRY used twice the "used" attribute Compiler Attributes: remove uses of __attribute__ from compiler.h Compiler Attributes: add Doc/process/programming-language.rst Compiler Attributes: add MAINTAINERS entry Compiler Attributes: add support for __nonstring (gcc >= 8) Compiler Attributes: enable -Wstringop-truncation on W=1 (gcc >= 8) Compiler Attributes: auxdisplay: panel: use __nonstring Compiler Attributes: ext4: remove local __nonstring definition ndesaulni...@google.com (2): compiler.h: update definition of unreachable() compiler-gcc: remove comment about gcc 4.5 from unreachable() Documentation/process/index.rst| 1 + Documentation/process/programming-language.rst | 45 + MAINTAINERS| 5 + drivers/auxdisplay/panel.c | 7 +- fs/ext4/ext4.h | 9 - include/linux/compiler-clang.h | 5 - include/linux/compiler-gcc.h | 74 +-- include/linux/compiler-intel.h | 9 - include/linux/compiler.h | 24 +-- include/linux/compiler_attributes.h| 258 + include/linux/compiler_types.h | 101 ++ scripts/Makefile.extrawarn | 1 + 12 files changed, 345 insertions(+), 194 deletions(-) create mode 100644 Documentation/process/programming-language.rst create mode 100644 include/linux/compiler_attributes.h
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:41 AM, Miguel Ojeda wrote: > On Mon, Oct 22, 2018 at 11:36 AM Kees Cook wrote: >> >> We need to make sure the static analyzers are happy with either >> method. Additionally, when was -Wimplicit-fallthrough added to GCC? If >> it was added _before_ the attribute, we need to continue using the >> comment style otherwise we lose coverage even with gcc itself. >> Additionally, does Clang support this attribute (it supports >> -Wimplicit-fallthrough). > > Please take a look at the rationale (also more details at the linked thread): > > * gcc 7.1 added -Wimplicit-fallthrough at the same time as the > attribute and the comment parsing. Ah, perfect. I missed this. :) > * clang does *not* support the attribute in C. Well that's not good. :) -Kees -- Kees Cook Pixel Security
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 12:45:03PM +0200, Bernd Petrovitsch wrote: > It's more like > snip > case 3: > frob(); > __fall_through; > /* no break - fall through */ > case 4: > snip > as "eclipse" doesn't accept anything else. > > And yes, it's far from "beautiful" but I hadn't time to dig into > eclipses innards to fix that. > Doing both is super ugly. Let's just do comments until Eclipse gets updated. I had wanted to move to the attribute because that would simplify things in Smatch but it's not a huge deal to delay for another year. regards, dan carpenter
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On 22/10/18 12:27, Dan Carpenter wrote: [...] > What does that even mean? Use both the attribute and the comment until > Eclipse is updated? > > case 3: > frob(); > __fall_through; /* fall through */ > case 4: > > That seems like a wrong idea... It's more like snip case 3: frob(); __fall_through; /* no break - fall through */ case 4: snip as "eclipse" doesn't accept anything else. And yes, it's far from "beautiful" but I hadn't time to dig into eclipses innards to fix that. MfG, Bernd -- "I dislike type abstraction if it has no real reason. And saving on typing is not a good reason - if your typing speed is the main issue when you're coding, you're doing something seriously wrong." - Linus Torvalds pEpkey.asc Description: application/pgp-keys
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 11:41:56AM +0200, Bernd Petrovitsch wrote: > On 22/10/2018 00:27, Theodore Y. Ts'o wrote: > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > >> From the GCC manual: > >> > >> fallthrough > >> > >> The fallthrough attribute with a null statement serves as a > >> fallthrough statement. It hints to the compiler that a statement > >> that falls through to another case label, or user-defined label > >> in a switch statement is intentional and thus the > >> -Wimplicit-fallthrough > >> warning must not trigger. The fallthrough attribute may appear > >> at most once in each attribute list, and may not be mixed with > >> other attributes. It can only be used in a switch statement > >> (the compiler will issue an error otherwise), after a preceding > >> statement and before a logically succeeding case label, > >> or user-defined label. > >> > >> https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html > > > > Do we know if coverity understands the fallthrough attribute? One of > > the reasons why I started using /* fallthrough */ is because it kept > > Coverity happy. > > FWIW, current "eclipse" has the same "problem". > > > If the conversion from /* fallthrough */ to the __fallthrough__ > > attribute means that we start gethting a lot of Coverity warnings, > > We could keep both. What does that even mean? Use both the attribute and the comment until Eclipse is updated? case 3: frob(); __fall_through; /* fall through */ case 4: That seems like a wrong idea... regards, dan carpenter
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On 22/10/2018 00:27, Theodore Y. Ts'o wrote: > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: >> From the GCC manual: >> >> fallthrough >> >> The fallthrough attribute with a null statement serves as a >> fallthrough statement. It hints to the compiler that a statement >> that falls through to another case label, or user-defined label >> in a switch statement is intentional and thus the -Wimplicit-fallthrough >> warning must not trigger. The fallthrough attribute may appear >> at most once in each attribute list, and may not be mixed with >> other attributes. It can only be used in a switch statement >> (the compiler will issue an error otherwise), after a preceding >> statement and before a logically succeeding case label, >> or user-defined label. >> >> https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html > > Do we know if coverity understands the fallthrough attribute? One of > the reasons why I started using /* fallthrough */ is because it kept > Coverity happy. FWIW, current "eclipse" has the same "problem". > If the conversion from /* fallthrough */ to the __fallthrough__ > attribute means that we start gethting a lot of Coverity warnings, We could keep both. MfG, Bernd -- Bernd Petrovitsch Email : be...@petrovitsch.priv.at LUGA : http://www.luga.at
Re: [PATCH 2/2] Compiler Attributes: auxdisplay: panel: use __fallthrough
On Sun, Oct 21, 2018 at 8:11 PM Joe Perches wrote: > > On Sun, 2018-10-21 at 19:14 +0200, Miguel Ojeda wrote: > > Let gcc know these cases are meant to fall through to the next label > > by annotating them with the new __fallthrough statement attribute; > > and remove the comment since it conveys the same information > > (which was also parsed by gcc to suppress the warning). > > Instead of many individual conversion patches, > perhaps a script to do all the conversions at once. Note that this was only an example of the attribute (some people asked for an example when introducing another one, so I preemptively did it for this one). Doing the conversion (and how :-) I left it for afterwards, if people agree with the attribute. > > Maybe: > > pattern='(?:\/\*\s*fall(?:\s*|\s*\-\s*)thr(?:u|ough)\s*\*\/|\/\/\s*fall\s*thr(?:u|ough)\s*$)' > git grep -P -i --name-only "$pattern" -- '*.[ch]' | > xargs perl -p -i -e "s/$pattern/__fallthrough;/gi" By the way, I checked first if coccinelle could match input comments, but it doesn't, according to Julia. I am also thinking whether a compiler plugin could easily do this, but I don't have my hopes high given these are comments... Also, regardless of how it is done, the patches need to be sent individually to maintainers, no? I have a vague memory that big & automated conversions were a bit frozen upon in the kernel. Greg...? Cheers, Miguel
Re: [PATCH 0/2] Compiler Attributes: __fallthrough
On Sun, Oct 21, 2018 at 8:29 PM Greg Kroah-Hartman wrote: > > On Sun, Oct 21, 2018 at 07:14:12PM +0200, Miguel Ojeda wrote: > > These are two patches are meant to go on top of the rest of the compiler > > attributes series on: > > > > https://github.com/ojeda/linux/tree/compiler-attributes > > > > which will be sent to Greg for the next merge window. > > > > Please review them and let me know! (specially if someone is against > > __fallthrough for some reason :-). > > Will this work with all of the static tools that are currently looking > for the comment instead? I know coverity handles that, what about > others? Thank you Greg, good question. I will try to keep all the information we can get about the tools in the commit message. I will also contact the different tools about this. Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 11:41 AM Miguel Ojeda wrote: > > * clang does *not* support the attribute in C. ...nor the warning at all, by the way (which is why this is OK). Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 11:36 AM Kees Cook wrote: > > We need to make sure the static analyzers are happy with either > method. Additionally, when was -Wimplicit-fallthrough added to GCC? If > it was added _before_ the attribute, we need to continue using the > comment style otherwise we lose coverage even with gcc itself. > Additionally, does Clang support this attribute (it supports > -Wimplicit-fallthrough). Please take a look at the rationale (also more details at the linked thread): * gcc 7.1 added -Wimplicit-fallthrough at the same time as the attribute and the comment parsing. * clang does *not* support the attribute in C. Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:26 AM, Miguel Ojeda wrote: > On Mon, Oct 22, 2018 at 12:27 AM Theodore Y. Ts'o wrote: >> >> On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: >> > From the GCC manual: >> > >> > fallthrough >> > >> > The fallthrough attribute with a null statement serves as a >> > fallthrough statement. It hints to the compiler that a statement >> > that falls through to another case label, or user-defined label >> > in a switch statement is intentional and thus the >> > -Wimplicit-fallthrough >> > warning must not trigger. The fallthrough attribute may appear >> > at most once in each attribute list, and may not be mixed with >> > other attributes. It can only be used in a switch statement >> > (the compiler will issue an error otherwise), after a preceding >> > statement and before a logically succeeding case label, >> > or user-defined label. >> > >> > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html Please CC Gustavo on these kinds of things -- he's been driving the bulk of the fall through coverage. >> Do we know if coverity understands the fallthrough attribute? One of >> the reasons why I started using /* fallthrough */ is because it kept >> Coverity happy. > > If Coverity is like gcc, they should be doing both (i.e. I see the > comment parsing as an "extra" that gcc did, but the "basic stuff" is > the attribute -- and I would guess it is way easier for them to > support than the comment parsing). > > But I cannot test it myself :-( Someone, please? > > However, if I understood Greg correctly in his reply to the cover > letter, he replied that Coverity knows about it (?). > >> >> If the conversion from /* fallthrough */ to the __fallthrough__ >> attribute means that we start gethting a lot of Coverity warnings, >> that would be unfortunate. OTOH, if this is getting standardized, >> maybe we can get Coverity to understand this attribute? > > Indeed! That would be the best for everyone, including Coverity customers. We need to make sure the static analyzers are happy with either method. Additionally, when was -Wimplicit-fallthrough added to GCC? If it was added _before_ the attribute, we need to continue using the comment style otherwise we lose coverage even with gcc itself. Additionally, does Clang support this attribute (it supports -Wimplicit-fallthrough). -Kees -- Kees Cook Pixel Security
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 2:42 AM Matthew Wilcox wrote: > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > +#if __has_attribute(__fallthrough__) > > +# define __fallthrough __attribute__((__fallthrough__)) > > +#else > > +# define __fallthrough > > +#endif > > Why is the #else not: > > # define __fallthrough /* fallthrough */ > > Would this solve the Coverity problem, or does Coverity look at the raw > source code before preprocessing? That wouldn't work if Coverity follows the standard, because it is required that comments are removed right before the preprocessing phase. That is one of the advantages vs. the attribute that I mentioned: """ We can actually use a #define for it like for the rest of attributes/extensions, which is not possible with a comment, (...) """ Cheers, Miguel
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 12:27 AM Theodore Y. Ts'o wrote: > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > From the GCC manual: > > > > fallthrough > > > > The fallthrough attribute with a null statement serves as a > > fallthrough statement. It hints to the compiler that a statement > > that falls through to another case label, or user-defined label > > in a switch statement is intentional and thus the -Wimplicit-fallthrough > > warning must not trigger. The fallthrough attribute may appear > > at most once in each attribute list, and may not be mixed with > > other attributes. It can only be used in a switch statement > > (the compiler will issue an error otherwise), after a preceding > > statement and before a logically succeeding case label, > > or user-defined label. > > > > https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html > > Do we know if coverity understands the fallthrough attribute? One of > the reasons why I started using /* fallthrough */ is because it kept > Coverity happy. If Coverity is like gcc, they should be doing both (i.e. I see the comment parsing as an "extra" that gcc did, but the "basic stuff" is the attribute -- and I would guess it is way easier for them to support than the comment parsing). But I cannot test it myself :-( Someone, please? However, if I understood Greg correctly in his reply to the cover letter, he replied that Coverity knows about it (?). > > If the conversion from /* fallthrough */ to the __fallthrough__ > attribute means that we start gethting a lot of Coverity warnings, > that would be unfortunate. OTOH, if this is getting standardized, > maybe we can get Coverity to understand this attribute? Indeed! That would be the best for everyone, including Coverity customers. Cheers, Miguel
Re: [RESEND PATCH v5 0/9] extend PWM framework to support PWM modes
Hello Claudiu, On Tue, Aug 28, 2018 at 04:01:17PM +0300, Claudiu Beznea wrote: > Please give feedback on these patches which extends the PWM framework in > order to support multiple PWM modes of operations. This series is a rework > of [1] and [2]. > > The current patch series add the following PWM modes: > - PWM mode normal > - PWM mode complementary > - PWM mode push-pull > > Normal mode - for PWM channels with one output; output waveforms looks like > this: > ________ > PWM __| |__| |__| |__| |__ > <--T--> > > Where T is the signal period In my discussion about some suggested changes to the PWM framework I used slightly different way to show the wave-forms in ASCII which are IMHO slightly better. Also I think it is valuable to not use a 50% duty cycle in the examples to remove some ambiguity. With a duty cycle of 1/3 the normal mode looks as follows in "my" way: __ __ __ PWM __/ \_/ \_/ \_/ ^^^^ The carets mark always the start of a period. And note the rising and falling edges are already part of the active and inactive phases respectively which matches reality. > Since PWMs with more than one output per channel could be used as one > output PWM the normal mode is the default mode for all PWMs (if not > specified otherwise). > > Complementary mode - for PWM channels with two outputs; output waveforms > for a PWM channel in complementary mode looks line this: > ________ > PWMH1 __| |__| |__| |__| |__ > __________ > PWML1 |__| |__| |__| |__| > <--T--> > > Where T is the signal period. So this translates to (I think): __ __ __ __ __ PWMH __/ \_/ \_/ \_/ \_/ \_/ _______ PWML \__/ \__/ \__/ \__/ \__/ \ ^^^^^^ That is PWML always pulls in the opposite direction of PWMH. Maybe we could come up with better terms than PWMH and PWML (which might be specific for the Atmel implementation?). Maybe "normal" and "complement"? > Push-pull mode - for PWM channels with two outputs; output waveforms for a > PWM channel in push-pull mode with normal polarity looks like this: > __ __ > PWMH __| || | > __ __ > PWML | || |__ ><--T--> That again with the alternative display method and duty cycle 1/3: ______ PWMA __/ \__/ \__/ \__ ____ PWMB ___/ \__/ \__/ ^^^^^^ That is PWMA and PWMB are active only every 2nd period taking alternate turns, right? > If polarity is inversed: > __ > PWMH |__||__| > __ > PWML |__||__| ><--T--> That's again with duty cycle 1/3: ________ PWMA \__/ \__/ \__/ _______ PWMB \__/ \__/ \ ^^^^^^ Given that the start of period isn't externally visible this is equivalent to using a duty cycle 2/3 and not inverting which results in: ________ PWMA \__/ \__/ \__/ _______ PWMB \__/ \__/ \ ^^^^^ I would really like if a more detailed description of the modes would be created as part of this series. Currently there are a few implied properties hidden in the PWM API (unrelated to this series) which I try to resolve together with Thierry. Getting this documented right from the start would be great here. I didn't look in detail into the driver implementation, but from the PWMs implemented in the STM32F4 family I would have chosen a different model which makes me wonder if we should stick to a more general way to describe two outputs from a single PWM channel. I would use four values with nanosecond resolution to describe these: .period .duty_cycle .alt_duty_cycle .alt_offset period and duty_cycle is as before for the primary output and then the alt_* values describe offset and duty cycle of the secondary output. What you called "normal mode" would then be specified using .period = $period .duty_cycle = $duty_cycle .alt_duty_cycle = 0 .alt_offset = dontcare Your "push pull mode" would be: .period = 2 * $period .duty_cycle = $duty_cycle .alt_dut
Re: [PATCH 1/2] Compiler Attributes: add support for __fallthrough (gcc >= 7.1)
On Mon, Oct 22, 2018 at 02:58:00AM -0400, Theodore Y. Ts'o wrote: > On Sun, Oct 21, 2018 at 05:42:18PM -0700, Matthew Wilcox wrote: > > On Sun, Oct 21, 2018 at 07:14:13PM +0200, Miguel Ojeda wrote: > > > +#if __has_attribute(__fallthrough__) > > > +# define __fallthrough __attribute__((__fallthrough__)) > > > +#else > > > +# define __fallthrough > > > +#endif > > > > Why is the #else not: > > > > # define __fallthrough /* fallthrough */ > > > > Would this solve the Coverity problem, or does Coverity look at the raw > > source code before preprocessing? > > Wouldn't the "/* ... */" be eaten by the preprocessor before defining > the __fallthrough cpp macro? (e.g., try running the attached script) You're right, even on older versions (4.7 here) : $ echo -e '#define foobar quux /* foobar */\nfoobar\n' | gcc -E - # 1 "" # 1 "" # 1 "" quux Willy
