Re: [OSS-Tools] [PATCH dt-utils] dtblint: add support for fsl, imx8mp-iomuxc
Hello Uwe, On 02.04.24 17:08, Uwe Kleine-König wrote: >>> + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_0" >>> regoffset=0x4c0 reset_default=0x */ >>> + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_1" >>> regoffset=0x4c4 reset_default=0x */ >>> + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_2" >>> regoffset=0x4c8 reset_default=0x */ >>> + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_3" >>> regoffset=0x4cc reset_default=0x */ >> >> [snip] >> >>> + /* regname="USDHC3_STROBE_SELECT_INPUT" regoffset=0x630 >>> reset_default=0x */ >>> + /* regname="USDHC3_WP_ON_SELECT_INPUT" regoffset=0x634 >>> reset_default=0x */ >> >> What are these comments about? > > The table of swmux and swpad registers originates from the reference > manual. These comments are the registers in the IOMUXC that are not > relevant for the linter. I kept them because copying these details from > the NXP pdf isn't trivial. I can drop them if they are considered to be > useless here. You can leave them in, but please add a leading comment saying that these pins are not muxable, so it's clear why they are commented out. Thanks, Ahmad > > Best regards > Uwe > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH v2] dtblint: add support for fsl, imx8mp-iomuxc
Hello Uwe,
On 02.04.24 16:51, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König
please address the other v1 feedback and mention what changes
between revisions.
Thanks,
Ahmad
> ---
> src/dtblint-imx-pinmux.c | 1315 ++
> 1 file changed, 1315 insertions(+)
>
> diff --git a/src/dtblint-imx-pinmux.c b/src/dtblint-imx-pinmux.c
> index 1fa2add28f85..02ed50af4b59 100644
> --- a/src/dtblint-imx-pinmux.c
> +++ b/src/dtblint-imx-pinmux.c
> @@ -4312,6 +4312,1318 @@ static const struct socinfo imx6q_socinfo = {
> .size_padinfo = ARRAY_SIZE(imx6q_iomux_padinfo),
> };
>
> +static const struct padinfo imx8mp_iomux_padinfo[] = {
> + {
> + .padname = "BOOT_MODE0",
> + .swpad_regoffset = 0x250,
> + .swpad_reset_default = 0x0194,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "BOOT_MODE1",
> + .swpad_regoffset = 0x254,
> + .swpad_reset_default = 0x0194,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "BOOT_MODE2",
> + .swpad_regoffset = 0x258,
> + .swpad_reset_default = 0x0194,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "BOOT_MODE3",
> + .swpad_regoffset = 0x25c,
> + .swpad_reset_default = 0x0194,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "JTAG_MOD",
> + .swpad_regoffset = 0x260,
> + .swpad_reset_default = 0x0114,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "JTAG_TDI",
> + .swpad_regoffset = 0x264,
> + .swpad_reset_default = 0x0154,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "JTAG_TMS",
> + .swpad_regoffset = 0x268,
> + .swpad_reset_default = 0x0154,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "JTAG_TCK",
> + .swpad_regoffset = 0x26c,
> + .swpad_reset_default = 0x0154,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "JTAG_TDO",
> + .swpad_regoffset = 0x270,
> + .swpad_reset_default = 0x0154,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO00",
> + .swmux_regoffset = 0x014,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x274,
> + .swpad_reset_default = 0x0106,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO01",
> + .swmux_regoffset = 0x018,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x278,
> + .swpad_reset_default = 0x0106,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO02",
> + .swmux_regoffset = 0x01c,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x27c,
> + .swpad_reset_default = 0x0146,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO03",
> + .swmux_regoffset = 0x020,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x280,
> + .swpad_reset_default = 0x0106,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO04",
> + .swmux_regoffset = 0x024,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x284,
> + .swpad_reset_default = 0x0106,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO05",
> + .swmux_regoffset = 0x028,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x288,
> + .swpad_reset_default = 0x0146,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO06",
> + .swmux_regoffset = 0x02c,
> + .swmux_reset_default = 0x,
> + .swmux_writeable_mask = 0x0017,
> + .swpad_regoffset = 0x28c,
> + .swpad_reset_default = 0x0106,
> + .swpad_writeable_mask = 0x01f6,
> + }, {
> + .padname = "GPIO1_IO07",
> + .swmux_regoffset = 0x030,
> +
Re: [OSS-Tools] [PATCH dt-utils] dtblint: add support for fsl, imx8mp-iomuxc
Hello Uwe, thanks for your patch. On 02.04.24 15:09, Uwe Kleine-König wrote: > Signed-off-by: Uwe Kleine-König Please add the version of the reference manual you took these values from. > --- > src/dtblint-imx-pinmux.c | 1317 ++ > 1 file changed, 1317 insertions(+) > > + .padname = "SD1_DATA5", > + .swmux_regoffset = 0x0a8, > + .swmux_reset_default = 0x0005, > + .swmux_writeable_mask = 0x0017, > + .swpad_regoffset = 0x308, > + .swpad_reset_default = 0x0106, > + .swpad_writeable_mask = 0x01f6, > + .swpad_writeable_mask = 0x01f6, This line is duplicated. > + .padname = "ECSPI2_SS0", > + .swmux_regoffset = 0x1fc, > + .swmux_reset_default = 0x0005, > + .swmux_writeable_mask = 0x0017, > + .swpad_regoffset = 0x45c, > + .swpad_reset_default = 0x0106, > + .swpad_writeable_mask = 0x01f6, > + .swpad_writeable_mask = 0x01f6, This is also duplicated. > + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_0" > regoffset=0x4c0 reset_default=0x */ > + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_1" > regoffset=0x4c4 reset_default=0x */ > + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_2" > regoffset=0x4c8 reset_default=0x */ > + /* regname="AUDIOMIX_PDM_MIC_PDM_BITSTREAM_SELECT_INPUT_3" > regoffset=0x4cc reset_default=0x */ [snip] > + /* regname="USDHC3_STROBE_SELECT_INPUT" regoffset=0x630 > reset_default=0x */ > + /* regname="USDHC3_WP_ON_SELECT_INPUT" regoffset=0x634 > reset_default=0x */ What are these comments about? Cheers, Ahmad -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [dt-utils] Release 2023.11.0
On 24.11.23 12:04, Roland Hieber wrote: > Hi, > > I've just release dt-utils version 2023.11.0. You can download it at the > usual location: > > https://public.pengutronix.de/software/dt-utils/dt-utils-2023.08.0.tar.xz https://public.pengutronix.de/software/dt-utils/dt-utils-2023.11.0.tar.xz > > The file checksums are as follows: > > * MD5:4aa4ef310c76a2baa5df62254f0b7453 > * SHA1: 3d9556c78c3894ef13fbf9b58eb66d1c11950563 > * SHA256: d224d941c076c143f43d59cd7c6e1c522926064a31ac34a67720632ddecb6b53 Hashes are correct though. Thanks for the release! Ahmad > > A small bugfix release, mainly consisting of the following commit: > > * commit b67e96895f52 "libdt: prefer first found disk when looking for > block devices" > When using barebox-state on boards with a barbeox-state partition on > an eMMC device, it could happen that barebox-state tried to find a GPT > partition on /dev/mmcblkXbootN instead of /dev/mmcblkX, obviously > failing to find a matching partition there, and quitting without any > further action. This commit restores the old behaviour of using the > first found MMC device (/dev/mmcblkX) instead of the last one. > Although this is not completely future-proof, it has worked for years > and serves a good stop-gap solution for now. > > A few more housekeeping and maintenance commits: > > * commit feca1c1ffdaa "README: provide git format.subjectPrefix line to copy" > * commit 1cd62596dbd7 "meson: align libdt-utils version with autotools'" > > This release includes contributions by Ahmad Fatoum, Enrico Jörns, Leonard > Göhrs, and Roland Hieber. Thank you to all contributors! > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH dt-utils v2] meson: align libdt-utils version with autotools'
Makefile.am has following defines for libtool versioning:
LIBDT_CURRENT=6
LIBDT_REVISION=0
LIBDT_AGE=0
along with a comment on how the values were chosen. Copy the comment and
the values into the meson.build as well, so appropriate symlinks
pointing at the versioned library are created. We forego the extra
complexity of having a common file that's read from both build systems
as we are intent on phasing out autotools anyway, once wrinkles such as
what's fixed here are ironed out.
The translation from libtool versioning to major/minor/revision as
expected by meson is taken from GNOME's gcab[1]. More information
about libtool versioning is available in its documentation[2].
[1]: https://gitlab.gnome.org/GNOME/gcab/-/commit/2c8048f74ec8c088397
[2]:
https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
Reported-by: Enrico Jörns
Signed-off-by: Ahmad Fatoum
---
v1 -> v2:
- set library version major as current - age
---
meson.build | 13 +
1 file changed, 13 insertions(+)
diff --git a/meson.build b/meson.build
index 9579e712dfea..6489e4c4d52e 100644
--- a/meson.build
+++ b/meson.build
@@ -122,6 +122,18 @@ versiondep = declare_dependency(sources: version_h)
meson.add_dist_script('version-gen', meson.project_version())
+# If the library source code has changed at all since the last release,
+# then increment revision (‘c:r:a’ becomes ‘c:r+1:a’).
+# If any interfaces have been added/removed/changed since the last release,
+# then increment current, and set revision to 0.
+# If any public interfaces have been added since the last public release,
+# then increment age.
+# If any interfaces have been removed or changed since the last release,
+# then set age to 0.
+lt_current = 6
+lt_revision = 0
+lt_age = 0
+
mapfile = 'src/libdt-utils.sym'
libdt_ld_flags =
'-Wl,--version-script,@0@/@1@'.format(meson.current_source_dir(), mapfile)
@@ -133,6 +145,7 @@ libdt = shared_library('dt-utils',
c_args : ['-include', meson.current_build_dir() / 'version.h'],
dependencies : [udevdep, versiondep],
gnu_symbol_visibility : 'default',
+ version: '@0@.@1@.@2@'.format(lt_current - lt_age, lt_age, lt_revision),
install : true)
executable('barebox-state',
--
2.39.2
Re: [OSS-Tools] [PATCH dt-utils] meson: align libdt-utils version with autotools'
Hello Enrico,
On 06.11.23 13:12, Enrico Jörns wrote:
> Am Montag, dem 06.11.2023 um 13:04 +0100 schrieb Roland Hieber:
>> On Mon, Nov 06, 2023 at 12:58:22PM +0100, Ahmad Fatoum wrote:
>>> Makefile.am has:
>>>
>>> LIBDT_CURRENT=6
>>> LIBDT_REVISION=0
>>> LIBDT_AGE=0
>>>
>>> along with a comment on how the values were chosen. Copy the comment and
>>> the values into the meson.build as well, so appropriate symlinks
>>> pointing at the versioned library are created. We forego the extra
>>> complexity of having a common file that's read from both build systems
>>> as we are intent on phasing out autotools anyway, once wrinkles such as
>>> what's fixed here are ironed out.
>>>
>>> Reported-by: Enrico Jörns
>>> Signed-off-by: Ahmad Fatoum
>>
>> Reviewed-by: Roland Hieber
>>
>>> ---
>>> meson.build | 9 +
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 9579e712dfea..9894d7311bb3 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -133,6 +133,15 @@ libdt = shared_library('dt-utils',
>>> c_args : ['-include', meson.current_build_dir() / 'version.h'],
>>> dependencies : [udevdep, versiondep],
>>> gnu_symbol_visibility : 'default',
>>> +# If the library source code has changed at all since the last release,
>>> +# then increment revision (‘c:r:a’ becomes ‘c:r+1:a’).
>>> +# If any interfaces have been added/removed/changed since the last release,
>>> +# then increment current, and set revision to 0.
>
> Does this make sense?
It was copied into Makefile.am verbatim from the libtool documentation:
https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> Isn't "current" meant to mark *incompatible* public API changes?
libtool numbering is new to me as well, but apparently "Current" is the most
recent
interface number that this library implements.
The major version number should thus be current - age, not just current.
I will fix that in v2. See
https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
https://gitlab.gnome.org/GNOME/gcab/-/merge_requests/6/diffs?commit_id=2c8048f74ec8c088397d47730aa47c574526918f
> This might be valid for "removed" or "changed", but not for "added".
>
> How does "interfaces" differ from "public interfaces" below?
No difference AFAIK.
Cheers,
Ahmad
>
> Regards, Enrico
>
>>> +# If any public interfaces have been added since the last public release,
>>> +# then increment age.
>>> +# If any interfaces have been removed or changed since the last release,
>>> +# then set age to 0.
>>> + version: '6.0.0',
>>> install : true)
>>>
>>> executable('barebox-state',
>>> --
>>> 2.39.2
>>>
>>>
>>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH dt-utils] meson: align libdt-utils version with autotools'
Makefile.am has:
LIBDT_CURRENT=6
LIBDT_REVISION=0
LIBDT_AGE=0
along with a comment on how the values were chosen. Copy the comment and
the values into the meson.build as well, so appropriate symlinks
pointing at the versioned library are created. We forego the extra
complexity of having a common file that's read from both build systems
as we are intent on phasing out autotools anyway, once wrinkles such as
what's fixed here are ironed out.
Reported-by: Enrico Jörns
Signed-off-by: Ahmad Fatoum
---
meson.build | 9 +
1 file changed, 9 insertions(+)
diff --git a/meson.build b/meson.build
index 9579e712dfea..9894d7311bb3 100644
--- a/meson.build
+++ b/meson.build
@@ -133,6 +133,15 @@ libdt = shared_library('dt-utils',
c_args : ['-include', meson.current_build_dir() / 'version.h'],
dependencies : [udevdep, versiondep],
gnu_symbol_visibility : 'default',
+# If the library source code has changed at all since the last release,
+# then increment revision (‘c:r:a’ becomes ‘c:r+1:a’).
+# If any interfaces have been added/removed/changed since the last release,
+# then increment current, and set revision to 0.
+# If any public interfaces have been added since the last public release,
+# then increment age.
+# If any interfaces have been removed or changed since the last release,
+# then set age to 0.
+ version: '6.0.0',
install : true)
executable('barebox-state',
--
2.39.2
[OSS-Tools] [PATCH dt-utils] README: provide git format.subjectPrefix line to copy
The oss-tools list is shared between a number of projects, so we should always use a subject prefix to make it clear where the patch applies. The README already notes that and references the git man page, but it's more convenient to just provide a command line that can be directly copied and pasted. Cc: Bastian Krause Signed-off-by: Ahmad Fatoum --- README | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README b/README index d68e6dacae6c..c6ef99c10b00 100644 --- a/README +++ b/README @@ -44,8 +44,10 @@ The Git web view for this software can be found at: <https://git.pengutronix.de/cgit/tools/dt-utils> Any patches should be sent to the mailing list above. Please prefix your -subject with "[PATCH dt-utils]" (when sending patches with Git, see the -git-config manpage for the option format.subjectPrefix). +subject with "[PATCH dt-utils]". This can be configured in Git with: + +git config format.subjectPrefix "PATCH dt-utils" + Mails sent to this mailing list are also archived at <https://lore.pengutronix.de/oss-tools/> -- 2.39.2
[OSS-Tools] [PATCH] libdt: prefer first found disk when looking for block devices
Recent rework introduced a regression for state located in the eMMC
user area described by DT fixed partitions. Whereas before, the first
device with type "disk" was taken, dt-utils will now iterate over all
devices to try to find a matching GPT partition. If it doesn't find any,
it will instead take the last device with type "disk", which will be the
second boot partition for eMMC devices leading barebox-state to access
/dev/mmcblkXboot1 instead of /dev/mmcblkX.
Let's fix this regression by restoring the old behavior of preferring the
first disk. This may not be totally future proof, but it worked ok for
years and a better solution can always be added later with a regular
release.
Reported-by: Leonard Göhrs
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libdt.c b/src/libdt.c
index 650b40467587..72e8ab41e09b 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2301,7 +2301,7 @@ static int cdev_from_block_device(struct udev_device *dev,
devtype = udev_device_get_devtype(part);
if (!devtype)
continue;
- if (!strcmp(devtype, "disk")) {
+ if (!strcmp(devtype, "disk") && !best_match) {
best_match = part;
/* Should we try to find a matching partition first? */
--
2.39.2
Re: [OSS-Tools] [PATCH dt-utils v2 9/9] README: mention compatibility with the REUSE specification
On 31.07.23 11:11, Roland Hieber wrote: > Signed-off-by: Roland Hieber Acked-by: Ahmad Fatoum > --- > PATCH v2: new in series v2 > --- > README | 4 > 1 file changed, 4 insertions(+) > > diff --git a/README b/README > index 3f53b47c03f8..9a0ab79c0167 100644 > --- a/README > +++ b/README > @@ -75,3 +75,7 @@ This program is distributed in the hope that it will be > useful, but WITHOUT ANY > WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS > FOR A > PARTICULAR PURPOSE. See the GNU General Public License (the file COPYING > bundled with this software) for more details. > + > +The dt-utils project conforms to the REUSE specification. You can use the > +'reuse' tool to get a Software Bill of Materials in SPDX format. For more > +information, see <https://reuse.software/>. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH dt-utils v2 8/9] DCO: add SPDX license information
On 31.07.23 11:11, Roland Hieber wrote: > There is nothing prefabricated yet, so use a custom LicenseRef instead. > > Signed-off-by: Roland Hieber Acked-by: Ahmad Fatoum > --- > PATCH v2: new in series v2 > --- > DCO | 2 ++ > LICENSES/LicenseRef-DCO.txt | 2 ++ > 2 files changed, 4 insertions(+) > create mode 100644 LICENSES/LicenseRef-DCO.txt > > diff --git a/DCO b/DCO > index 8201f992154a..75e56dd2683f 100644 > --- a/DCO > +++ b/DCO > @@ -1,3 +1,5 @@ > +# SPDX-License-Identifier: LicenseRef-DCO > + > Developer Certificate of Origin > Version 1.1 > > diff --git a/LICENSES/LicenseRef-DCO.txt b/LICENSES/LicenseRef-DCO.txt > new file mode 100644 > index ..b5b12d7c360b > --- /dev/null > +++ b/LICENSES/LicenseRef-DCO.txt > @@ -0,0 +1,2 @@ > +Everyone is permitted to copy and distribute verbatim copies of this > +license document, but changing it is not allowed. -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH dt-utils v2 7/9] treewide: add trivial copyright headers
On 31.07.23 11:11, Roland Hieber wrote: > 'reuse lint' warns about files without copyright information, and > refuses to give us the spec compliance checkmark. Add trivial copyright > lines to all remaining files that don't have any yet. Acked-by: Ahmad Fatoum > > Signed-off-by: Roland Hieber > --- > PATCH v2: new in series v2 > --- > .gitignore | 1 + > Makefile.am | 1 + > NEWS| 1 + > autogen.sh | 1 + > configure.ac| 1 + > m4/.gitignore | 1 + > meson_options.txt | 1 + > scripts/barebox-mark-successful-boot.sh | 1 + > src/.gitignore | 1 + > src/asm/unaligned.h | 1 + > src/barebox-state.h | 2 ++ > src/barebox-state/state.h | 1 + > src/base64.h| 2 ++ > src/crc.h | 1 + > src/crypto/sha.h| 1 + > src/driver.h| 1 + > src/dt/common.h | 2 ++ > src/dt/dt.h | 2 ++ > src/dt/fdt.h| 2 ++ > src/dt/list.h | 2 ++ > src/dtblint.h | 1 + > src/fdt.h | 2 ++ > src/fdtdump.c | 2 ++ > src/fs.h| 1 + > src/init.h | 1 + > src/libbb.h | 2 ++ > src/libdt-utils.sym | 1 + > src/libfile.h | 1 + > src/linux/err.h | 1 + > src/linux/list.h| 1 + > src/linux/mtd/mtd-abi.h | 1 + > src/module.h| 1 + > src/mtd/mtd-peb.h | 1 + > src/net.h | 1 + > src/of.h| 1 + > src/printk.h| 1 + > src/state.h | 1 + > test/01-fixed-partition-no-gpt.dts | 1 + > test/02-fixed-partition-before-gpt-partition.dts| 1 + > test/03-fixed-partition-is-gpt-partition.dts| 1 + > test/04-gpt-partition-by-partuuid.dts | 1 + > test/05-gpt-partition-by-typeuuid.dts | 1 + > test/06-fixed-partition-by-diskuuid.dts | 1 + > test/07-raw-disk-fail.dts | 1 + > test/08-gpt-disk-no-typeuuid-fail.dts | 1 + > test/09-no-disk-fail.dts| 1 + > test/31-fixed-partition-overlaps-two-gpt-partitions.dts | 1 + > ...32-fixed-partition-overlaps-two-gpt-partitions-partially.dts | 1 + > test/33-fixed-partition-part-of-gpt-partition.dts | 1 + > test/barebox-state.dtsi | 1 + > test/barebox-state.t| 1 + > test/crc32.c| 1 + > test/gpt-no-typeuuid.config | 1 + > test/gpt.config | 1 + > test/meson.build| 1 + > test/raw.config | 1 + > version-gen | 1 + > version.h.in| 1 + > 58 files changed, 67 insertions(+) > > diff --git a/.gitignore b/.gitignore > index f6afc0defbfa..c7b8d9da6ac2 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -1,4 +1,5 @@ > # SPDX-Lic
Re: [OSS-Tools] [PATCH dt-utils v2 6/9] treewide: add GPL-2.0-only SPDX identifiers to files without license
On 31.07.23 11:11, Roland Hieber wrote: > The dt-utils project license has effectively been GPL-2.0-only since > commit 2b39a389428224d96bbb (2014-11-27, Sascha Hauer: "COPYING: Change > to GPLv2"). Reflect this in the file headers so the license stays with > the code in case files get copied around. > > Link: > https://git.pengutronix.de/cgit/tools/dt-utils/commit/?id=2b39a389428224d96bbb > Signed-off-by: Roland Hieber Reviewed-by: Ahmad Fatoum > --- > PATCH v2: > * rebase to next branch and add remaining files > > PATCH v1: > https://lore.pengutronix.org/oss-tools/20210326210647.8648-4-...@pengutronix.de > --- > .gitignore | 1 + > Makefile.am | 2 ++ > NEWS| 2 ++ > README | 2 ++ > autogen.sh | 1 + > configure.ac| 2 ++ > m4/.gitignore | 1 + > meson.build | 1 + > meson_options.txt | 1 + > scripts/barebox-mark-successful-boot.sh | 1 + > src/.gitignore | 1 + > src/barebox-state.h | 1 + > src/base64.h| 1 + > src/crypto/sha.h| 1 + > src/dt/common.h | 1 + > src/dt/dt.h | 1 + > src/dt/fdt.h| 1 + > src/dt/list.h | 1 + > src/dtblint.h | 1 + > src/fdt.h | 1 + > src/fdtdump.c | 1 + > src/fs.h| 1 + > src/libbb.h | 1 + > src/libdt-utils.sym | 1 + > src/linux/list.h| 1 + > src/linux/mtd/mtd-abi.h | 1 + > src/of.h| 1 + > src/printk.h| 1 + > src/state.h | 1 + > test/01-fixed-partition-no-gpt.dts | 1 + > test/02-fixed-partition-before-gpt-partition.dts| 1 + > test/03-fixed-partition-is-gpt-partition.dts| 1 + > test/04-gpt-partition-by-partuuid.dts | 1 + > test/05-gpt-partition-by-typeuuid.dts | 1 + > test/06-fixed-partition-by-diskuuid.dts | 1 + > test/07-raw-disk-fail.dts | 1 + > test/08-gpt-disk-no-typeuuid-fail.dts | 1 + > test/09-no-disk-fail.dts| 1 + > test/31-fixed-partition-overlaps-two-gpt-partitions.dts | 1 + > ...32-fixed-partition-overlaps-two-gpt-partitions-partially.dts | 1 + > test/33-fixed-partition-part-of-gpt-partition.dts | 1 + > test/barebox-state.dtsi | 1 + > test/barebox-state.t| 1 + > test/crc32.c| 1 + > test/gpt-no-typeuuid.config | 1 + > test/gpt.config | 1 + > test/meson.build| 1 + > test/raw.config | 1 + > version-gen | 1 + > version.h.in| 1 + > 50 files changed, 54 insertions(+) > > diff --git a/.gitignore b/.gitignore > index 22060feceab3..f6afc0defbfa 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -1,3 +1,4 @@ > +# SPDX-License-Identifier: GPL-2.0-only > *~ > *.o > .deps/ > diff --git a/Makefile.am b/Makefile.am > index d3077317ce9a..960ac4902af5 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -1,3 +1,5 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + &g
Re: [OSS-Tools] [PATCH dt-utils v2 5/9] treewide: add CC0-1.0 SPDX identifiers for trivial files
On 31.07.23 11:11, Roland Hieber wrote:
> Add license text from the SPDX website [1].
>
> [1]: https://spdx.org/licenses/CC0-1.0.html
>
> Signed-off-by: Roland Hieber
I don't think it's necessary to add a SPDX-License-Identifier to empy files,
but I can live with it:
Acked-by: Ahmad Fatoum
> ---
> PATCH v2: new in series v2
> ---
> LICENSES/CC0-1.0.txt | 121 +++
> src/asm/unaligned.h | 1 +
> src/crc.h| 1 +
> src/driver.h | 1 +
> src/init.h | 1 +
> src/libfile.h| 1 +
> src/linux/err.h | 1 +
> src/module.h | 1 +
> src/mtd/mtd-peb.h| 1 +
> src/net.h| 1 +
> 10 files changed, 130 insertions(+)
> create mode 100644 LICENSES/CC0-1.0.txt
>
> diff --git a/LICENSES/CC0-1.0.txt b/LICENSES/CC0-1.0.txt
> new file mode 100644
> index ..0e259d42c996
> --- /dev/null
> +++ b/LICENSES/CC0-1.0.txt
> @@ -0,0 +1,121 @@
> +Creative Commons Legal Code
> +
> +CC0 1.0 Universal
> +
> +CREATIVE COMMONS CORPORATION IS NOT A LAW FIRM AND DOES NOT PROVIDE
> +LEGAL SERVICES. DISTRIBUTION OF THIS DOCUMENT DOES NOT CREATE AN
> +ATTORNEY-CLIENT RELATIONSHIP. CREATIVE COMMONS PROVIDES THIS
> +INFORMATION ON AN "AS-IS" BASIS. CREATIVE COMMONS MAKES NO WARRANTIES
> +REGARDING THE USE OF THIS DOCUMENT OR THE INFORMATION OR WORKS
> +PROVIDED HEREUNDER, AND DISCLAIMS LIABILITY FOR DAMAGES RESULTING FROM
> +THE USE OF THIS DOCUMENT OR THE INFORMATION OR WORKS PROVIDED
> +HEREUNDER.
> +
> +Statement of Purpose
> +
> +The laws of most jurisdictions throughout the world automatically confer
> +exclusive Copyright and Related Rights (defined below) upon the creator
> +and subsequent owner(s) (each and all, an "owner") of an original work of
> +authorship and/or a database (each, a "Work").
> +
> +Certain owners wish to permanently relinquish those rights to a Work for
> +the purpose of contributing to a commons of creative, cultural and
> +scientific works ("Commons") that the public can reliably and without fear
> +of later claims of infringement build upon, modify, incorporate in other
> +works, reuse and redistribute as freely as possible in any form whatsoever
> +and for any purposes, including without limitation commercial purposes.
> +These owners may contribute to the Commons to promote the ideal of a free
> +culture and the further production of creative, cultural and scientific
> +works, or to gain reputation or greater distribution for their Work in
> +part through the use and efforts of others.
> +
> +For these and/or other purposes and motivations, and without any
> +expectation of additional consideration or compensation, the person
> +associating CC0 with a Work (the "Affirmer"), to the extent that he or she
> +is an owner of Copyright and Related Rights in the Work, voluntarily
> +elects to apply CC0 to the Work and publicly distribute the Work under its
> +terms, with knowledge of his or her Copyright and Related Rights in the
> +Work and the meaning and intended legal effect of CC0 on those rights.
> +
> +1. Copyright and Related Rights. A Work made available under CC0 may be
> +protected by copyright and related or neighboring rights ("Copyright and
> +Related Rights"). Copyright and Related Rights include, but are not
> +limited to, the following:
> +
> + i. the right to reproduce, adapt, distribute, perform, display,
> + communicate, and translate a Work;
> + ii. moral rights retained by the original author(s) and/or performer(s);
> +iii. publicity and privacy rights pertaining to a person's image or
> + likeness depicted in a Work;
> + iv. rights protecting against unfair competition in regards to a Work,
> + subject to the limitations in paragraph 4(a), below;
> + v. rights protecting the extraction, dissemination, use and reuse of data
> + in a Work;
> + vi. database rights (such as those arising under Directive 96/9/EC of the
> + European Parliament and of the Council of 11 March 1996 on the legal
> + protection of databases, and under any national implementation
> + thereof, including any amended or successor version of such
> + directive); and
> +vii. other similar, equivalent or corresponding rights throughout the
> + world based on applicable law or treaty, and any national
> + implementations thereof.
> +
> +2. Waiver. To the greatest extent permitted by, but not in contravention
> +of, applicable law, Affirmer hereby overtly, fully, permanently,
> +irrevocably and unconditionally waives, abandons, and surrenders all of
> +Affirmer's Copyright and Related
Re: [OSS-Tools] [PATCH dt-utils v2 4/9] treewide: add SPDX identifier to file with Zlib license
On 31.07.23 11:11, Roland Hieber wrote: > We don't have any zlib.h anyway, so the line in src/crc32.c is > meaningless. Add Zlib license from the SPDX website [1]. > > [1]: https://spdx.org/licenses/Zlib.html > > Reviewed-by: Uwe Kleine-König > Signed-off-by: Roland Hieber Reviewed-by: Ahmad Fatoum > --- > PATCH v2: > * add LICENSES/Zlib.txt > > PATCH v1: > https://lore.pengutronix.org/oss-tools/20210326210647.8648-3-...@pengutronix.de > --- > LICENSES/Zlib.txt | 19 +++ > src/crc32.c | 2 +- > 2 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 LICENSES/Zlib.txt > > diff --git a/LICENSES/Zlib.txt b/LICENSES/Zlib.txt > new file mode 100644 > index ..4dd43e509207 > --- /dev/null > +++ b/LICENSES/Zlib.txt > @@ -0,0 +1,19 @@ > +zlib License > + > +This software is provided 'as-is', without any express or implied warranty. > In > +no event will the authors be held liable for any damages arising from the > use of > +this software. > + > +Permission is granted to anyone to use this software for any purpose, > including > +commercial applications, and to alter it and redistribute it freely, subject > to > +the following restrictions: > + > +1. The origin of this software must not be misrepresented; you must not claim > + that you wrote the original software. If you use this software in a > product, > + an acknowledgment in the product documentation would be appreciated but is > + not required. > + > +2. Altered source versions must be plainly marked as such, and must not be > + misrepresented as being the original software. > + > +3. This notice may not be removed or altered from any source distribution. > diff --git a/src/crc32.c b/src/crc32.c > index 8273d3465f6f..469dac3a5b27 100644 > --- a/src/crc32.c > +++ b/src/crc32.c > @@ -1,3 +1,4 @@ > +/* SPDX-License-Identifier: Zlib */ > /* > * This file is derived from crc32.c from the zlib-1.1.3 distribution > * by Jean-loup Gailly and Mark Adler. > @@ -5,7 +6,6 @@ > > /* crc32.c -- compute the CRC-32 of a data stream > * Copyright (C) 1995-1998 Mark Adler > - * For conditions of distribution and use, see copyright notice in zlib.h > */ > > #include -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH dt-utils v2 3/9] treewide: add SPDX identifiers to files with GPL-3.0-or-later license
On 31.07.23 11:11, Roland Hieber wrote: > Use GPL-3.0-or-later license text from the SPDX website [1], and add the > boilerplate headers. > > [1]: https://spdx.org/licenses/GPL-3.0-or-later > > Signed-off-by: Roland Hieber Acked-by: Ahmad Fatoum > --- > PATCH v2: new in series v2 > --- > LICENSES/GPL-3.0-or-later.txt | 672 ++ > check-news.sh | 2 +- > 2 files changed, 673 insertions(+), 1 deletion(-) > create mode 100644 LICENSES/GPL-3.0-or-later.txt > > diff --git a/LICENSES/GPL-3.0-or-later.txt b/LICENSES/GPL-3.0-or-later.txt > new file mode 100644 > index ..45edb014fd52 > --- /dev/null > +++ b/LICENSES/GPL-3.0-or-later.txt > @@ -0,0 +1,672 @@ > +This program is free software; you can redistribute it and/or modify > +it under the terms of the GNU General Public License as published by > +the Free Software Foundation; either version 3 of the License, or > +(at your option) any later version. > + > +This program is distributed in the hope that it will be useful, > +but WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +GNU General Public License for more details. > + > + > +GNU GENERAL PUBLIC LICENSE > +Version 3, 29 June 2007 > + > +Copyright © 2007 Free Software Foundation, Inc. <http://fsf.org/> > + > +Everyone is permitted to copy and distribute verbatim copies of this > +license document, but changing it is not allowed. > + > +Preamble > + > +The GNU General Public License is a free, copyleft license for software > +and other kinds of works. > + > +The licenses for most software and other practical works are designed to > +take away your freedom to share and change the works. By contrast, the > +GNU General Public License is intended to guarantee your freedom to > +share and change all versions of a program--to make sure it remains free > +software for all its users. We, the Free Software Foundation, use the > +GNU General Public License for most of our software; it applies also to > +any other work released this way by its authors. You can apply it to > +your programs, too. > + > +When we speak of free software, we are referring to freedom, not price. > +Our General Public Licenses are designed to make sure that you have the > +freedom to distribute copies of free software (and charge for them if > +you wish), that you receive source code or can get it if you want it, > +that you can change the software or use pieces of it in new free > +programs, and that you know you can do these things. > + > +To protect your rights, we need to prevent others from denying you these > +rights or asking you to surrender the rights. Therefore, you have > +certain responsibilities if you distribute copies of the software, or if > +you modify it: responsibilities to respect the freedom of others. > + > +For example, if you distribute copies of such a program, whether gratis > +or for a fee, you must pass on to the recipients the same freedoms that > +you received. You must make sure that they, too, receive or can get the > +source code. And you must show them these terms so they know their > +rights. > + > +Developers that use the GNU GPL protect your rights with two steps: (1) > +assert copyright on the software, and (2) offer you this License giving > +you legal permission to copy, distribute and/or modify it. > + > +For the developers' and authors' protection, the GPL clearly explains > +that there is no warranty for this free software. For both users' and > +authors' sake, the GPL requires that modified versions be marked as > +changed, so that their problems will not be attributed erroneously to > +authors of previous versions. > + > +Some devices are designed to deny users access to install or run > +modified versions of the software inside them, although the manufacturer > +can do so. This is fundamentally incompatible with the aim of protecting > +users' freedom to change the software. The systematic pattern of such > +abuse occurs in the area of products for individuals to use, which is > +precisely where it is most unacceptable. Therefore, we have designed > +this version of the GPL to prohibit the practice for those products. If > +such problems arise substantially in other domains, we stand ready to > +extend this provision to those domains in future versions of the GPL, as > +needed to protect the freedom of users. > + > +Finally, every program is threatened constantly by software patents. > +States should not allow patents to restrict development and use of > +software on general-purpose computers, but in those that do, we wish to > +avoid the special danger that
Re: [OSS-Tools] [PATCH dt-utils v2 2/9] treewide: add SPDX identifiers to files with GPL-2.0-or-later license
On 31.07.23 11:11, Roland Hieber wrote: > Use the same license text as for GPL-2.0-only, but with different > boilerplate headers. > > Signed-off-by: Roland Hieber Acked-by: Ahmad Fatoum > -- > PATCH v2: > * add LICENSES/GPL-2.0-or-later.txt > * don't remove boilerplate license headers, see my own comment: > > https://lore.pengutronix.org/oss-tools/20210331100305.e4klllzxx4p7y...@pengutronix.de >and the REUSE FAQ: https://reuse.software/faq/#edit-copyright-and-licensing > > PATCH v1: > https://lore.pengutronix.org/oss-tools/20210326210647.8648-1-...@pengutronix.de > --- > LICENSES/GPL-2.0-or-later.txt | 351 > src/barebox-state/state.c | 1 + > src/barebox-state/state_variables.c | 1 + > src/base64.c| 3 +- > src/crypto/sha1.c | 1 + > src/crypto/sha2.c | 1 + > src/linux/uuid.h| 1 + > test/sharness.sh| 1 + > 8 files changed, 358 insertions(+), 2 deletions(-) > create mode 100644 LICENSES/GPL-2.0-or-later.txt > > diff --git a/LICENSES/GPL-2.0-or-later.txt b/LICENSES/GPL-2.0-or-later.txt > new file mode 100644 > index ..7fe1b477516e > --- /dev/null > +++ b/LICENSES/GPL-2.0-or-later.txt > @@ -0,0 +1,351 @@ > +This program is free software: you can redistribute it and/or modify > +it under the terms of the GNU General Public License as published by > +the Free Software Foundation, either version 2 of the License, or > +(at your option) any later version. > + > +This program is distributed in the hope that it will be useful, > +but WITHOUT ANY WARRANTY; without even the implied warranty of > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +GNU General Public License for more details. > + > + > + GNU GENERAL PUBLIC LICENSE > +Version 2, June 1991 > + > + Copyright (C) 1989, 1991 Free Software Foundation, Inc. > + 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + Everyone is permitted to copy and distribute verbatim copies > + of this license document, but changing it is not allowed. > + > + Preamble > + > + The licenses for most software are designed to take away your > +freedom to share and change it. By contrast, the GNU General Public > +License is intended to guarantee your freedom to share and change free > +software--to make sure the software is free for all its users. This > +General Public License applies to most of the Free Software > +Foundation's software and to any other program whose authors commit to > +using it. (Some other Free Software Foundation software is covered by > +the GNU Library General Public License instead.) You can apply it to > +your programs, too. > + > + When we speak of free software, we are referring to freedom, not > +price. Our General Public Licenses are designed to make sure that you > +have the freedom to distribute copies of free software (and charge for > +this service if you wish), that you receive source code or can get it > +if you want it, that you can change the software or use pieces of it > +in new free programs; and that you know you can do these things. > + > + To protect your rights, we need to make restrictions that forbid > +anyone to deny you these rights or to ask you to surrender the rights. > +These restrictions translate to certain responsibilities for you if you > +distribute copies of the software, or if you modify it. > + > + For example, if you distribute copies of such a program, whether > +gratis or for a fee, you must give the recipients all the rights that > +you have. You must make sure that they, too, receive or can get the > +source code. And you must show them these terms so they know their > +rights. > + > + We protect your rights with two steps: (1) copyright the software, and > +(2) offer you this license which gives you legal permission to copy, > +distribute and/or modify the software. > + > + Also, for each author's protection and ours, we want to make certain > +that everyone understands that there is no warranty for this free > +software. If the software is modified by someone else and passed on, we > +want its recipients to know that what they have is not the original, so > +that any problems introduced by others will not reflect on the original > +authors' reputations. > + > + Finally, any free program is threatened constantly by software > +patents. We wish to avoid the danger that redistributors of a free > +program will individually obtain patent licenses, in effect making the > +program proprietary. To prevent this, we have made it c
Re: [OSS-Tools] [PATCH dt-utils v2 1/9] treewide: add SPDX identifiers to files with GPL-2.0-only license
On 31.07.23 11:11, Roland Hieber wrote: > Move the GPL-2.0 license text to the new LICENSES folder, where it will > reside next to all other licenses. Add the rest of the license too (see > FSF FAQ [1]) and clean up the hard page breaks in the process. > > Add the standard license dedication and warranty disclaimer too for > completeness sake, so we can easily reference the full license terms by > using its SPDX identifier, and don't have to carry around the > boilerplate headers in the future. However, keep the already existing > license headers in files, as recommended in the REUSE FAQ [2]. > > "COPYING" is probably still referenced somewhere, so make it point to > the new location of the GPL. > > Link: [1] https://www.gnu.org/licenses/gpl-faq.html#GPLOmitPreamble > Link: [2] https://reuse.software/faq/#edit-copyright-and-licensing > Signed-off-by: Roland Hieber Reviewed-by: Ahmad Fatoum > -- > PATCH v2: > * move COPYING to LICENSES/GPL-2.0-only.txt > * use the complete GPL-2.0 text > * don't remove boilerplate license headers from files > > PATCH v1: > https://lore.pengutronix.org/oss-tools/20210326210647.8648-2-...@pengutronix.de > --- > COPYING | 281 +--- > LICENSES/GPL-2.0-only.txt | 350 > src/barebox-state.c | 1 + > src/barebox-state/backend_bucket_circular.c | 1 + > src/barebox-state/backend_bucket_direct.c | 1 + > src/barebox-state/backend_format_dtb.c | 1 + > src/barebox-state/backend_format_raw.c | 1 + > src/barebox-state/backend_storage.c | 1 + > src/crypto/digest.c | 1 + > src/crypto/hmac.c | 3 +- > src/crypto/internal.h | 3 +- > src/crypto/keystore.h | 1 + > src/digest.h| 1 + > src/dtblint-imx-pinmux.c| 1 + > src/dtblint.c | 1 + > src/fdt.c | 1 + > src/keystore-blob.c | 1 + > src/libdt.c | 1 + > 18 files changed, 367 insertions(+), 284 deletions(-) > mode change 100644 => 12 COPYING > create mode 100644 LICENSES/GPL-2.0-only.txt > > diff --git a/COPYING b/COPYING > deleted file mode 100644 > index 5a965fbc58c9.. > --- a/COPYING > +++ /dev/null > @@ -1,280 +0,0 @@ > - GNU GENERAL PUBLIC LICENSE > -Version 2, June 1991 > - > - Copyright (C) 1989, 1991 Free Software Foundation, Inc. > - 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > - Everyone is permitted to copy and distribute verbatim copies > - of this license document, but changing it is not allowed. > - > - Preamble > - > - The licenses for most software are designed to take away your > -freedom to share and change it. By contrast, the GNU General Public > -License is intended to guarantee your freedom to share and change free > -software--to make sure the software is free for all its users. This > -General Public License applies to most of the Free Software > -Foundation's software and to any other program whose authors commit to > -using it. (Some other Free Software Foundation software is covered by > -the GNU Library General Public License instead.) You can apply it to > -your programs, too. > - > - When we speak of free software, we are referring to freedom, not > -price. Our General Public Licenses are designed to make sure that you > -have the freedom to distribute copies of free software (and charge for > -this service if you wish), that you receive source code or can get it > -if you want it, that you can change the software or use pieces of it > -in new free programs; and that you know you can do these things. > - > - To protect your rights, we need to make restrictions that forbid > -anyone to deny you these rights or to ask you to surrender the rights. > -These restrictions translate to certain responsibilities for you if you > -distribute copies of the software, or if you modify it. > - > - For example, if you distribute copies of such a program, whether > -gratis or for a fee, you must give the recipients all the rights that > -you have. You must make sure that they, too, receive or can get the > -source code. And you must show them these terms so they know their > -rights. > - > - We protect your rights with two steps: (1) copyright the software, and > -(2) offer you this license which gives you legal permission to copy, > -distribute and/or modify the software. > - > -
Re: [OSS-Tools] [PATCH dt-utils] README: clarify the need for "real names" with the DCO process
On 31.07.23 10:51, Roland Hieber wrote: > The Linux kernel recently changed the wording of "real names" to "known > identities" (see commit d4563201f33a022fc035, 2023-02-26, Linus > Torvalds: "Documentation: simplify and clarify DCO contribution example > language"). In fact, we already credit contributors in our Git history > by names that are probably not their "real names". Follow the reasoning > in the kernel guidelines in that regard, and adapt our wording > accordingly. > > Link: https://git.kernel.org/torvalds/c/d4563201f33a022fc035 > Signed-off-by: Roland Hieber Acked-by: Ahmad Fatoum > --- > README | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/README b/README > index acfd2c171286..428c459f7f79 100644 > --- a/README > +++ b/README > @@ -57,8 +57,8 @@ By adding a Signed-off-by line (e.g. using `git commit -s`) > saying > >Signed-off-by: Random J Developer > > -(using your real name and e-mail address), you state that your contributions > -are in line with the DCO. > +(using your known identity), you state that your contributions are in line > with > +the DCO. > > License > --- -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH 2/3] test: add test case with non-existent /dev/file
From: Ahmad Fatoum
When backend points at a device that couldn't be resolved, barebox-state
should fail instead of taking another device.
This is meant to address issues like the one fixed by commit
e7d71f099659 ("libdt: fix of_get_devicepath looking up sibling if
device unavailable"), but the error case there can't be reproduced
exactly, because loop devices have no common parent. Still one test
is better than no test until we start testing in Qemu.
Signed-off-by: Ahmad Fatoum
---
test/09-no-disk-fail.dts | 26 ++
1 file changed, 26 insertions(+)
create mode 100644 test/09-no-disk-fail.dts
diff --git a/test/09-no-disk-fail.dts b/test/09-no-disk-fail.dts
new file mode 100644
index ..49cfa852525d
--- /dev/null
+++ b/test/09-no-disk-fail.dts
@@ -0,0 +1,26 @@
+/dts-v1/;
+
+#include "barebox-state.dtsi"
+
+/ {
+ expected-dev = "";
+
+ disk: loopfile0 {
+ compatible = "barebox,hostfile";
+ barebox,filename = "/dev/barebox-state-dev-does-not-exist";
+ barebox,blockdev;
+ };
+
+ otherdisk: loopfile1 {
+ compatible = "barebox,hostfile";
+ barebox,filename = __GPT_LOOPDEV__;
+ barebox,blockdev;
+ };
+};
+
+ {
+ backend = <>;
+ backend-type = "raw";
+ backend-stridesize = <0x40>;
+ backend-storage-type = "direct";
+};
--
2.39.2
[OSS-Tools] [PATCH 1/3] meson: options: use defaults of type boolean for boolean options
From: Ahmad Fatoum Building with newer meson versions alerts us that specifying defaults with a string type for non-string types is deprecated. This type confusion was not intentional, so fix that. Signed-off-by: Ahmad Fatoum --- meson_options.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/meson_options.txt b/meson_options.txt index f80643aa6c73..04d9854a4b1b 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -2,24 +2,24 @@ option( 'state-backward-compatibility', type : 'boolean', - value : 'false', + value : false, description : 'barebox-state: when using the "direct" storage backend, keep the on-disk format readable by barebox <= v2016.08.0') option( 'lock-device', type : 'boolean', - value : 'false', + value : false, description : 'lock device node instead of creating lockfile in /run') # build options option( 'barebox-state', type : 'boolean', - value : 'true', + value : true, description : 'Build barebox-state utility') option( 'tests', type : 'boolean', - value : 'true', + value : true, description : 'Enable/Disable test suite') -- 2.39.2
[OSS-Tools] [PATCH 3/3] meson: set optimization level to -O2 by default
From: Ahmad Fatoum We shouldn't expect users to override optimization levels, so let's have a sensible default. Signed-off-by: Ahmad Fatoum --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index be92446f137a..e03e1dbf6e85 100644 --- a/meson.build +++ b/meson.build @@ -9,6 +9,7 @@ project( default_options: [ 'c_std=gnu11', 'warning_level=2', +'optimization=2', ], license : 'GPL-2.0-only', ) -- 2.39.2
Re: [OSS-Tools] [PATCH 0/5] Add meson support and first test suite
On 05.06.23 12:17, Roland Hieber wrote: > On Wed, May 31, 2023 at 05:31:20PM +0200, Ahmad Fatoum wrote: >> The barebox,state binding is quite complex and we have a lot of udev >> parsing code that can only be exercised by manually running >> barebox-state on the target. Make development less error prone, by >> adding tests for the block device bindings. EEPROM and MTD can >> follow later. >> >> Tests are executed by meson as a runner. Patches to teach autotools >> to do the same are welcome, although I think we should follow RAUC's >> steps and eventually deprecate autotools once meson is on par. > > Yes. > >> The obvious wart is that we build with -fvisibility=hidden on autotools, >> but with meson the same visibility option results in linker errors. >> >> I have no idea why yet, but that should only make meson-built >> libdt-utils a bit slower without functional change. >> >> Ahmad Fatoum (5): >> Add meson as build system >> state: add option to lock device node >> meson: add simple integration test >> libdt: add CONFIG_TEST_LOOPBACK >> test: add barebox-state loop block device tests > > For all: > Tested-by: Roland Hieber Thanks. Applied to next. > >> >> .gitignore| 2 + >> README| 21 + >> check-news.sh | 82 ++ >> configure.ac | 11 + >> meson.build | 163 >> meson_options.txt | 25 + >> src/barebox-state.c | 30 +- >> src/barebox-state/state.c | 4 + >> src/barebox-state/state.h | 21 + >> src/dt/dt.h | 1 - >> src/libdt.c | 50 +- >> test/01-fixed-partition-no-gpt.dts| 34 + >> ...2-fixed-partition-before-gpt-partition.dts | 34 + >> test/03-fixed-partition-is-gpt-partition.dts | 34 + >> test/04-gpt-partition-by-partuuid.dts | 31 + >> test/05-gpt-partition-by-typeuuid.dts | 23 + >> test/06-fixed-partition-by-diskuuid.dts | 33 + >> test/07-raw-disk-fail.dts | 18 + >> test/08-gpt-disk-no-typeuuid-fail.dts | 18 + >> ...-partition-overlaps-two-gpt-partitions.dts | 34 + >> ...-overlaps-two-gpt-partitions-partially.dts | 34 + >> ...-fixed-partition-part-of-gpt-partition.dts | 34 + >> test/barebox-state.dtsi | 53 ++ >> test/barebox-state.t | 229 + >> test/crc32.c | 18 + >> test/gpt-no-typeuuid.config | 33 + >> test/gpt.config | 35 + >> test/meson.build | 36 + >> test/raw.config | 24 + >> test/sharness.sh | 857 ++ >> version-gen | 3 + >> version.h.in | 3 + >> 32 files changed, 2012 insertions(+), 16 deletions(-) >> create mode 100755 check-news.sh >> create mode 100644 meson.build >> create mode 100644 meson_options.txt >> create mode 100644 test/01-fixed-partition-no-gpt.dts >> create mode 100644 test/02-fixed-partition-before-gpt-partition.dts >> create mode 100644 test/03-fixed-partition-is-gpt-partition.dts >> create mode 100644 test/04-gpt-partition-by-partuuid.dts >> create mode 100644 test/05-gpt-partition-by-typeuuid.dts >> create mode 100644 test/06-fixed-partition-by-diskuuid.dts >> create mode 100644 test/07-raw-disk-fail.dts >> create mode 100644 test/08-gpt-disk-no-typeuuid-fail.dts >> create mode 100644 test/31-fixed-partition-overlaps-two-gpt-partitions.dts >> create mode 100644 >> test/32-fixed-partition-overlaps-two-gpt-partitions-partially.dts >> create mode 100644 test/33-fixed-partition-part-of-gpt-partition.dts >> create mode 100644 test/barebox-state.dtsi >> create mode 100755 test/barebox-state.t >> create mode 100644 test/crc32.c >> create mode 100644 test/gpt-no-typeuuid.config >> create mode 100644 test/gpt.config >> create mode 100644 test/meson.build >> create mode 100644 test/raw.config >> create mode 100755 test/sharness.sh >> create mode 100755 version-gen >> create mode 100644 version.h.in >> >> -- >> 2.39.2 >> >> >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH 4/5] libdt: add CONFIG_TEST_LOOPBACK
On 31.05.23 17:31, Ahmad Fatoum wrote:
> We have quite a bit of tricky udev and device tree parsing code that is
> only tested manually. Best case we would have tests for this in QEMU,
> but until we do, let's test it with loop devices:
>
> The downside is that we can only test block devices and that we need
> a tiny bit of code that's not used normally, but on the upside, we
> have tests. :-)
>
> Signed-off-by: Ahmad Fatoum
> ---
> configure.ac | 2 ++
> meson.build | 1 +
> src/dt/dt.h | 1 -
> src/libdt.c | 50 +++---
> 4 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 117a1e169ba9..5b5c74c2b582 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -40,6 +40,8 @@ AC_DEFINE(CONFIG_MTD, [1], [Statically define to be enabled
> to harmonize barebox
>
> AC_DEFINE(CONFIG_STATE, [1], [Statically define to be enabled to harmonize
> barebox' & dt-utils' code base.])
>
> +AC_DEFINE(CONFIG_TEST_LOOPBACK, [0], [Only enabled in meson for testing.])
> +
> AC_CHECK_FUNCS([__secure_getenv secure_getenv])
>
> my_CFLAGS="-Wall \
> diff --git a/meson.build b/meson.build
> index 1bc32274af07..be92446f137a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -21,6 +21,7 @@ conf.set10('CONFIG_MTD', true)
> conf.set10('CONFIG_STATE', true)
> conf.set10('CONFIG_STATE_BACKWARD_COMPATIBLE',
> get_option('state-backward-compatibility'))
> conf.set10('CONFIG_LOCK_DEVICE_NODE', get_option('lock-device'))
> +conf.set10('CONFIG_TEST_LOOPBACK', get_option('tests'))
>
> meson.add_dist_script(
>find_program('check-news.sh').path(),
> diff --git a/src/dt/dt.h b/src/dt/dt.h
> index a4213d49606a..153b56e09a21 100644
> --- a/src/dt/dt.h
> +++ b/src/dt/dt.h
> @@ -220,7 +220,6 @@ extern int of_set_root_node(struct device_node *node);
> extern int of_platform_populate(struct device_node *root,
> const struct of_device_id *matches,
> struct device_d *parent);
> -extern struct device_d *of_find_device_by_node(struct device_node *np);
>
> int of_device_is_stdout_path(struct device_d *dev);
> const char *of_get_model(void);
> diff --git a/src/libdt.c b/src/libdt.c
> index af28de6f17c6..7c24c9197c7f 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2091,6 +2091,22 @@ struct udev_of_path {
>
> static LIST_HEAD(udev_of_devices);
>
> +static const char *udev_device_get_of_path(struct udev_device *dev)
> +{
> + const char *of_path;
> +
> + of_path = udev_device_get_property_value(dev, "OF_FULLNAME");
> + if (of_path)
> + return of_path;
> +
> + if (IS_ENABLED(CONFIG_TEST_LOOPBACK) &&
> + !strcmp(udev_device_get_subsystem(dev), "block") &&
> + !strncmp(udev_device_get_sysname(dev), "loop", 4))
> + return udev_device_get_sysname(dev);
> +
> + return NULL;
> +}
> +
> static void of_scan_udev_devices(void)
> {
> struct udev *udev;
> @@ -2111,6 +2127,8 @@ static void of_scan_udev_devices(void)
> udev_enumerate_add_match_subsystem(enumerate, "spi");
> udev_enumerate_add_match_subsystem(enumerate, "mtd");
> udev_enumerate_add_match_subsystem(enumerate, "amba");
> + if (IS_ENABLED(CONFIG_TEST_LOOPBACK))
> + udev_enumerate_add_match_subsystem(enumerate, "block");
> udev_enumerate_scan_devices(enumerate);
> devices = udev_enumerate_get_list_entry(enumerate);
>
> @@ -2124,7 +2142,7 @@ static void of_scan_udev_devices(void)
> path = udev_list_entry_get_name(dev_list_entry);
> dev = udev_device_new_from_syspath(udev, path);
>
> - of_path = udev_device_get_property_value(dev, "OF_FULLNAME");
> + of_path = udev_device_get_of_path(dev);
> if (!of_path)
> continue;
>
> @@ -2148,11 +2166,37 @@ struct udev_device *of_find_device_by_node_path(const
> char *of_full_path)
> ret = udev_of_path->udev;
> break;
> }
> + if (!strcmp(udev_of_path->of_path, of_full_path)) {
> + ret = udev_of_path->udev;
> + break;
> + }
This if clause is duplicated. Will remove when applying.
> }
>
> return ret;
> }
>
> +static struct udev_device *of_find_device_by_node(struct device_node *np)
> +{
> + struct udev_of_path *udev_of_path;
> + struct udev_device *dev;
> + co
Re: [OSS-Tools] [PATCH v2 0/8] state: allow lookup of barebox state partition by Type GUID
On 07.06.23 14:16, Ahmad Fatoum wrote: > This implements the binding extension introduced to barebox here: > https://lore.barebox.org/barebox/20230607120714.3083182-1-a.fat...@pengutronix.de/T/#t > > With this, barebox,state backend can optionally point at a device > instead of a partition. If this device is GPT-partitioned and has > a partition with a specific partition type GUID of > > 4778ed65-bf42-45fa-9c5b-287a1dc4aab1 > > It will be taken. > > This series also fixes an annoying issue of barebox-state triggering > udev on every access, because the root block device corresponding > to the device tree node was opened r/w. > > barebox-state will now open the disk read-only if possible and if > a partition exists that fits the barebox state location, it will > be opened instead. Pulled into dt-utils next branch. Patches are already in barebox-next. > > v1 -> v2: > - added Uwe's Reviewed-by > - fix typo spotted by Roland > - Rebase on top of next > - Sync with v2 of barebox series: > - fix typo (Sascha) > - define new cdev_find_child_by_gpt_typeuuid helper (Marco) > - handle case of iterating over partitions before disk in > cdev_from_block_device > > Ahmad Fatoum (8): > state: backend: direct: open block device in read-only mode if > possible > libdt: factor out u64 sysattr parsing into helper > libdt: drop broken if-branch > libdt: factor out __of_cdev_find helper > libdt: use block device partition instead of parent if found > state: align with barebox use of of_cdev_find > libdt: use of_find_device_by_uuid for partuuid lookup > state: allow lookup of barebox state partition by Type GUID > > src/barebox-state/backend_bucket_direct.c | 5 +- > src/barebox-state/backend_storage.c | 2 +- > src/barebox-state/state.c | 52 +++- > src/barebox-state/state.h | 3 +- > src/dt/common.h | 8 + > src/dt/dt.h | 7 + > src/libdt-utils.sym | 4 + > src/libdt.c | 336 ++ > src/linux/uuid.h | 24 ++ > src/state.h | 4 + > 10 files changed, 368 insertions(+), 77 deletions(-) > create mode 100644 src/linux/uuid.h > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH v2 3/8] libdt: drop broken if-branch
device_find_block_device returns 0 on success, so the way the else if
clause is now, only if there is a block device, the code falls through.
If there is none, a 0 is returned, but devpath is not populated breaking
the contract of the function. Just drop the branch for now and add it back
later in a way that works.
Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robust")
Reviewed-by: Uwe Kleine-König
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 3f4595707554..a4f0e256ef6a 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2519,13 +2519,10 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
*/
dev = of_find_device_by_node_path(partition_node->full_name);
if (dev) {
- if (udev_device_is_eeprom(dev)) {
+ if (udev_device_is_eeprom(dev))
return udev_parse_eeprom(dev, devpath);
- } else if (!udev_parse_mtd(dev, devpath, size)) {
+ if (!udev_parse_mtd(dev, devpath, size))
return 0;
- } else if (device_find_block_device(dev, devpath)) {
- return of_parse_partition(partition_node, offset, size);
- }
/*
* If we find a udev_device but couldn't classify it above,
--
2.39.2
[OSS-Tools] [PATCH v2 7/8] libdt: use of_find_device_by_uuid for partuuid lookup
With the addition of of_find_device_by_uuid as part of the support for
barebox,storage-by-uuid, we don't need to rely on the device symlinks
created by 60-persistent-storage.rules and instead can just use libudev
directly, which we do elsewhere anyway. This has the added benefit
that we unify with the other code paths, where we determine the device
path through the struct udev_device.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index ae98543400b6..a55bc9b8d27d 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2560,18 +2560,22 @@ static int __of_cdev_find(struct device_node
*partition_node, struct cdev *cdev)
/* when partuuid is specified short-circuit the search for the
cdev */
ret = of_property_read_string(partition_node, "partuuid",
);
if (!ret) {
- const char prefix[] = "/dev/disk/by-partuuid/";
- size_t prefix_len = sizeof(prefix) - 1;
- char *lc_uuid, *s;
+ u64 partsize;
+ int ret;
- lc_uuid = xzalloc(prefix_len + strlen(uuid) + 1);
- s = memcpy(lc_uuid, prefix, prefix_len) + prefix_len;
+ dev = of_find_device_by_uuid(NULL, uuid, false);
+ if (!dev) {
+ fprintf(stderr, "%s: cannot find device for
uuid %s\n", __func__,
+ uuid);
+ return -ENODEV;
+ }
- while (*uuid)
- *s++ = tolower(*uuid++);
-
- cdev->devpath = lc_uuid;
+ ret = udev_device_parse_sysattr_u64(dev, "size",
);
+ if (ret)
+ return -EINVAL;
+ cdev->size = partsize * 512;
+ cdev->devpath = strdup(udev_device_get_devnode(dev));
return 0;
}
}
--
2.39.2
[OSS-Tools] [PATCH v2 5/8] libdt: use block device partition instead of parent if found
Linux doesn't parse MTD fixed-partitions described in the device tree
for block devices and EEPROMs. For block devices, The dt-utils work around
this by doing the lookup in userspace and then arrive at the parent block
device along with an offset that is passed along. We can do better
though: If there's a partition block device that contains the region
we are interested in, we should just be using that.
This avoids the issue of triggering a reread of the partition table
after writing barebox state because udev is notified that we had
been opening the whole block device writable.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 60 +++--
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index c8a8ea43e452..1b381ed8915b 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2212,23 +2212,29 @@ static int udev_device_parse_sysattr_u64(struct
udev_device *dev, const char *at
return 0;
}
+/* True if A completely contains B */
+static bool region_contains(loff_t starta, loff_t enda,
+ loff_t startb, loff_t endb)
+{
+return starta <= startb && enda >= endb;
+}
+
/*
- * device_find_block_device - extract device path from udev block device
+ * cdev_from_block_device - Populate cdev from udev block device
*
* @dev: the udev_device to extract information from
- * @devpath: returns the devicepath under which the block device is
accessible
+ * @cdev: the cdev output parameter to populate
*
* returns 0 for success or negative error value on failure.
*/
-int device_find_block_device(struct udev_device *dev,
- char **devpath)
+static int cdev_from_block_device(struct udev_device *dev,
+ struct cdev *cdev)
{
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- struct udev_device *part;
- const char *outpath;
+ struct udev_device *part, *best_match = NULL;
int ret;
udev = udev_new();
@@ -2252,19 +2258,43 @@ int device_find_block_device(struct udev_device *dev,
if (!devtype)
continue;
if (!strcmp(devtype, "disk")) {
- outpath = udev_device_get_devnode(part);
- *devpath = strdup(outpath);
- ret = 0;
- goto out;
+ best_match = part;
+
+ /* Should we try to find a matching partition first? */
+ if (!cdev->size)
+ break;
+ } else if (cdev->size && !strcmp(devtype, "partition")) {
+ u64 partstart, partsize;
+
+ ret = udev_device_parse_sysattr_u64(part, "start",
);
+ if (ret)
+ continue;
+
+ ret = udev_device_parse_sysattr_u64(part, "size",
);
+ if (ret)
+ continue;
+
+ /* start/size sys attributes are always in 512-byte
units */
+ partstart *= 512;
+ partsize *= 512;
+
+ if (!region_contains(partstart, partstart + partsize,
+cdev->offset, cdev->offset +
cdev->size))
+ continue;
+
+ best_match = part;
+ cdev->offset -= partstart;
+ break;
}
}
- ret = -ENODEV;
-out:
+ if (best_match)
+ cdev->devpath = strdup(udev_device_get_devnode(best_match));
+
udev_enumerate_unref(enumerate);
udev_unref(udev);
- return ret;
+ return best_match ? 0 : -ENODEV;
}
/*
@@ -2606,10 +2636,10 @@ static int __of_cdev_find(struct device_node
*partition_node, struct cdev *cdev)
return of_parse_partition(partition_node, >offset,
>size);
} else {
- ret = device_find_block_device(dev, >devpath);
+ ret = of_parse_partition(partition_node, >offset,
>size);
if (ret)
return ret;
- return of_parse_partition(partition_node, >offset,
>size);
+ return cdev_from_block_device(dev, cdev);
}
return -EINVAL;
--
2.39.2
[OSS-Tools] [PATCH v2 2/8] libdt: factor out u64 sysattr parsing into helper
We will need to parse two more sysattrs in a follow-up patch, so factor
this out for reuse. While at it switch to 64-bit strtoull: This may
be more useful for future users and unlike atol doesn't invoke undefined
behavior when parsing fails.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 42 +++---
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 9534c7635f34..3f4595707554 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2179,6 +2179,33 @@ static struct udev_device
*device_find_mtd_partition(struct udev_device *dev,
return NULL;
}
+/*
+ * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit
integer
+ * @dev: the udev_device to extract the attribute from
+ * @attr: name of the attribute to lookup
+ * @outvalue: returns the value parsed out of the attribute
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char
*attr,
+ u64 *outvalue)
+{
+ char *endptr;
+ u64 value;
+ const char *str;
+
+ str = udev_device_get_sysattr_value(dev, attr);
+ if (!str)
+ return -EINVAL;
+
+ value = strtoull(str, , 0);
+ if (!*str || *endptr)
+ return -EINVAL;
+
+ *outvalue = value;
+ return 0;
+}
+
/*
* device_find_block_device - extract device path from udev block device
*
@@ -2305,24 +2332,25 @@ static int udev_device_is_eeprom(struct udev_device
*dev)
* udev_parse_mtd - get information from a mtd udev_device
* @dev: the udev_device to extract information from
* @devpath: returns the devicepath under which the mtd device is accessible
- * @size: returns the size of the mtd device
+ * @outsize: returns the size of the mtd device
*
* returns 0 for success or negative error value on failure. *devpath
* will be valid on success and must be freed after usage.
*/
-static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*size)
+static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*outsize)
{
- const char *sizestr;
const char *outpath;
+ u64 size;
+ int ret;
if (!udev_device_is_mtd(dev))
return -EINVAL;
- sizestr = udev_device_get_sysattr_value(dev, "size");
- if (!sizestr)
- return -EINVAL;
+ ret = udev_device_parse_sysattr_u64(dev, "size", );
+ if (ret)
+ return ret;
- *size = atol(sizestr);
+ *outsize = size;
outpath = udev_device_get_devnode(dev);
if (!outpath)
--
2.39.2
[OSS-Tools] [PATCH v2 6/8] state: align with barebox use of of_cdev_find
As part of barebox-state by GPT partition type GUID lookup support,
state code in barebox needs direct interaction with the cdev, so it can
enumerate the child partitions. We have recently added __of_cdev_find
with internal linkage, so lets export of_cdev_find with external linkage
for outside use. Unlike __of_cdev_find, the new external function will
return dynamically allocated memory to avoid consumer code having to
know the struct cdev layout.
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/state.c | 26 +++---
src/dt/common.h | 8
src/dt/dt.h | 3 +++
src/libdt-utils.sym | 2 ++
src/libdt.c | 37 +
5 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 3174c84b8ded..29e04c3d7ea8 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -578,6 +578,20 @@ void state_release(struct state *state)
free(state);
}
+#ifdef __BAREBOX__
+static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
+{
+ /*
+* We only accept partitions exactly mapping the barebox-state,
+* but dt-utils may need to set non-zero values here
+*/
+ *offset = 0;
+ *size = 0;
+
+ return basprintf("/dev/%s", cdev->name);
+}
+#endif
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -594,8 +608,9 @@ struct state *state_new_from_node(struct device_node *node,
bool readonly)
const char *alias;
uint32_t stridesize;
struct device_node *partition_node;
- off_t offset = 0;
- size_t size = 0;
+ struct cdev *cdev;
+ off_t offset;
+ size_t size;
alias = of_alias_get(node);
if (!alias) {
@@ -614,11 +629,8 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
-#ifdef __BAREBOX__
- ret = of_find_path_by_node(partition_node, >backend_path, 0);
-#else
- ret = of_get_devicepath(partition_node, >backend_path, ,
);
-#endif
+ cdev = of_cdev_find(partition_node);
+ ret = PTR_ERR_OR_ZERO(cdev);
if (ret) {
if (ret != -EPROBE_DEFER)
dev_err(>dev, "state failed to parse path to
backend: %s\n",
diff --git a/src/dt/common.h b/src/dt/common.h
index d16f1193fbe3..38dc61cd65fe 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -166,6 +166,14 @@ static inline void *ERR_CAST(const void *ptr)
return (void *) ptr;
}
+static inline int PTR_ERR_OR_ZERO(const void *ptr)
+{
+ if (IS_ERR(ptr))
+ return PTR_ERR(ptr);
+ else
+ return 0;
+}
+
static inline char *barebox_asprintf(const char *fmt, ...) __attribute__
((format(__printf__, 1, 2)));
static inline char *barebox_asprintf(const char *fmt, ...)
{
diff --git a/src/dt/dt.h b/src/dt/dt.h
index d5e49eb32df9..f8bd3e56efed 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -231,6 +231,9 @@ void of_add_memory_bank(struct device_node *node, bool
dump, int r,
int of_get_devicepath(struct device_node *partition_node, char **devnode,
off_t *offset,
size_t *size);
+struct cdev *of_cdev_find(struct device_node *partition_node);
+char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
+
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index a749317fa024..f95c74305fb0 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -34,6 +34,8 @@ global:
of_get_child_by_name;
of_get_child_count;
of_get_devicepath;
+ of_cdev_find;
+ cdev_to_devpath;
of_get_next_available_child;
of_get_parent;
of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index 1b381ed8915b..ae98543400b6 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2679,3 +2679,40 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
return 0;
}
+
+/*
+ * of_cdev_find - get information how to access device corresponding to a
device_node
+ * @partition_node:The device_node which shall be accessed
+ * @devpath: Returns the devicepath under which the device is
accessible
+ * @offset:Returns the offset in the device
+ * @size: Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition.
+ * For this partition the function returns the device path and the offset
+ * and size in the device. For mtd devices the path will be /dev/mtdx, for
+ * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
+ * For mtd devices the device path returned will be the partition itself.
+ * Since EEPR
[OSS-Tools] [PATCH v2 0/8] state: allow lookup of barebox state partition by Type GUID
This implements the binding extension introduced to barebox here: https://lore.barebox.org/barebox/20230607120714.3083182-1-a.fat...@pengutronix.de/T/#t With this, barebox,state backend can optionally point at a device instead of a partition. If this device is GPT-partitioned and has a partition with a specific partition type GUID of 4778ed65-bf42-45fa-9c5b-287a1dc4aab1 It will be taken. This series also fixes an annoying issue of barebox-state triggering udev on every access, because the root block device corresponding to the device tree node was opened r/w. barebox-state will now open the disk read-only if possible and if a partition exists that fits the barebox state location, it will be opened instead. v1 -> v2: - added Uwe's Reviewed-by - fix typo spotted by Roland - Rebase on top of next - Sync with v2 of barebox series: - fix typo (Sascha) - define new cdev_find_child_by_gpt_typeuuid helper (Marco) - handle case of iterating over partitions before disk in cdev_from_block_device Ahmad Fatoum (8): state: backend: direct: open block device in read-only mode if possible libdt: factor out u64 sysattr parsing into helper libdt: drop broken if-branch libdt: factor out __of_cdev_find helper libdt: use block device partition instead of parent if found state: align with barebox use of of_cdev_find libdt: use of_find_device_by_uuid for partuuid lookup state: allow lookup of barebox state partition by Type GUID src/barebox-state/backend_bucket_direct.c | 5 +- src/barebox-state/backend_storage.c | 2 +- src/barebox-state/state.c | 52 +++- src/barebox-state/state.h | 3 +- src/dt/common.h | 8 + src/dt/dt.h | 7 + src/libdt-utils.sym | 4 + src/libdt.c | 336 ++ src/linux/uuid.h | 24 ++ src/state.h | 4 + 10 files changed, 368 insertions(+), 77 deletions(-) create mode 100644 src/linux/uuid.h -- 2.39.2
[OSS-Tools] [PATCH v2 4/8] libdt: factor out __of_cdev_find helper
of_get_devicepath is an exported symbol by libdt, so we shouldn't change
its signature or its semantics. Yet, we will want to export more
information out of the udev'ification code in future. already
declares an opaque struct cdev, so let's add a __of_cdev_find helper
that can populate it with the same information that of_get_devicepath
would return. In later commits we will define helpers that return
and process a struct cdev placed in dynamic memory.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 82 +
1 file changed, 52 insertions(+), 30 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index a4f0e256ef6a..c8a8ea43e452 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -32,6 +32,12 @@
#include
#include
+struct cdev {
+ char *devpath;
+ off_t offset;
+ size_t size;
+};
+
static int pr_level = 5;
void pr_level_set(int level)
@@ -2482,33 +2488,14 @@ static struct udev_device
*of_find_device_by_uuid(struct udev_device *parent,
return NULL;
}
-/*
- * of_get_devicepath - get information how to access device corresponding to a
device_node
- * @partition_node:The device_node which shall be accessed
- * @devpath: Returns the devicepath under which the device is
accessible
- * @offset:Returns the offset in the device
- * @size: Returns the size of the device
- *
- * This function takes a device_node which represents a partition.
- * For this partition the function returns the device path and the offset
- * and size in the device. For mtd devices the path will be /dev/mtdx, for
- * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
- * For mtd devices the device path returned will be the partition itself.
- * Since EEPROMs do not have partitions under Linux @offset and @size will
- * describe the offset and size inside the full device. The same applies to
- * block devices.
- *
- * returns 0 for success or negative error value on failure.
- */
-int of_get_devicepath(struct device_node *partition_node, char **devpath,
off_t *offset,
- size_t *size)
+static int __of_cdev_find(struct device_node *partition_node, struct cdev
*cdev)
{
struct device_node *node;
struct udev_device *dev, *partdev, *mtd;
int ret;
- *offset = 0;
- *size = 0;
+ cdev->offset = 0;
+ cdev->size = 0;
/*
* simplest case: This nodepath can directly be translated into
@@ -2520,8 +2507,8 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
dev = of_find_device_by_node_path(partition_node->full_name);
if (dev) {
if (udev_device_is_eeprom(dev))
- return udev_parse_eeprom(dev, devpath);
- if (!udev_parse_mtd(dev, devpath, size))
+ return udev_parse_eeprom(dev, >devpath);
+ if (!udev_parse_mtd(dev, >devpath, >size))
return 0;
/*
@@ -2553,7 +2540,7 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
while (*uuid)
*s++ = tolower(*uuid++);
- *devpath = lc_uuid;
+ cdev->devpath = lc_uuid;
return 0;
}
@@ -2609,21 +2596,56 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
return -ENODEV;
/* ...find the desired information by mtd udev_device */
- return udev_parse_mtd(partdev, devpath, size);
+ return udev_parse_mtd(partdev, >devpath, >size);
}
if (udev_device_is_eeprom(dev)) {
- ret = udev_parse_eeprom(dev, devpath);
+ ret = udev_parse_eeprom(dev, >devpath);
if (ret)
return ret;
- return of_parse_partition(partition_node, offset, size);
+ return of_parse_partition(partition_node, >offset,
>size);
} else {
- ret = device_find_block_device(dev, devpath);
+ ret = device_find_block_device(dev, >devpath);
if (ret)
return ret;
- return of_parse_partition(partition_node, offset, size);
+ return of_parse_partition(partition_node, >offset,
>size);
}
return -EINVAL;
}
+
+/*
+ * of_get_devicepath - get information how to access device corresponding to a
device_node
+ * @partition_node:The device_node which shall be accessed
+ * @devpath: Returns the devicepath under which the device is
accessible
+ * @offset:Returns the offset in the device
+ * @size: Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition
[OSS-Tools] [PATCH v2 8/8] state: allow lookup of barebox state partition by Type GUID
The backend device tree property so far always pointed at a partition.
Let's allow pointing it at GPT storage devices directly and lookup
the correct barebox state partition by the well-known type GUID:
4778ed65-bf42-45fa-9c5b-287a1dc4aab1
The implementation is not as neat as hoped for, because lifetime for
barebox cdev and libudev udev_device are quite different and libdt
doesn't yet get the latter right. Once we make sure not to leak
libudev objects and add cdev_unref stubs to barebox, this can be
further simplified by sticking the struct udev_device into struct
cdev and determining extra information on demand.
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/state.c | 20 +
src/dt/dt.h | 4 ++
src/libdt-utils.sym | 2 +
src/libdt.c | 94 ++-
src/linux/uuid.h | 24 ++
src/state.h | 4 ++
6 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 src/linux/uuid.h
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 29e04c3d7ea8..41256d79e3c2 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -592,6 +593,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t
*offset, size_t *size)
}
#endif
+static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -638,6 +641,23 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
+ /* Is the backend referencing an on-disk partitionable block device? */
+ if (cdev_is_block_disk(cdev)) {
+ cdev = cdev_find_child_by_gpt_typeuuid(cdev,
_state_partition_guid);
+ if (IS_ERR(cdev)) {
+ ret = -EINVAL;
+ goto out_release_state;
+ }
+
+ pr_debug("%s: backend GPT partition looked up via
PartitionTypeGUID\n",
+node->full_name);
+ }
+
+ state->backend_path = cdev_to_devpath(cdev, , );
+
+ pr_debug("%s: backend resolved to %s %lld %zu\n", node->full_name,
+state->backend_path, (long long)offset, size);
+
state->backend_reproducible_name =
of_get_reproducible_name(partition_node);
ret = of_property_read_string(node, "backend-type", _type);
diff --git a/src/dt/dt.h b/src/dt/dt.h
index f8bd3e56efed..f20f5680fc6c 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
/* Default string compare functions */
#define of_compat_cmp(s1, s2, l) strcasecmp((s1), (s2))
@@ -234,6 +235,9 @@ int of_get_devicepath(struct device_node *partition_node,
char **devnode, off_t
struct cdev *of_cdev_find(struct device_node *partition_node);
char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
+bool cdev_is_block_disk(struct cdev *cdev);
+struct cdev *cdev_find_child_by_gpt_typeuuid(struct cdev *cdev, guid_t
*typeuuid);
+
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index f95c74305fb0..636c3461e490 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -36,6 +36,8 @@ global:
of_get_devicepath;
of_cdev_find;
cdev_to_devpath;
+ cdev_is_block_disk;
+ cdev_find_child_by_gpt_typeuuid;
of_get_next_available_child;
of_get_parent;
of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index a55bc9b8d27d..5022bc7c7804 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -30,12 +30,15 @@
#include
#include
#include
+#include
#include
struct cdev {
char *devpath;
off_t offset;
size_t size;
+ bool is_gpt_partitioned;
+ bool is_block_disk;
};
static int pr_level = 5;
@@ -2288,8 +2291,13 @@ static int cdev_from_block_device(struct udev_device
*dev,
}
}
- if (best_match)
+ if (best_match) {
+ const char *part_type;
+
cdev->devpath = strdup(udev_device_get_devnode(best_match));
+ part_type = udev_device_get_property_value(best_match,
"ID_PART_TABLE_TYPE");
+ cdev->is_gpt_partitioned = part_type && !strcmp(part_type,
"gpt");
+ }
udev_enumerate_unref(enumerate);
udev_unref(udev);
@@ -2526,6 +2534,8 @@ static int __of_cdev_find(struct device_node
*partition_node, struct cdev *cdev)
cdev->offset = 0;
cdev->size = 0;
+ cdev->is_gpt_partitioned = false;
+ cdev->is_block_disk = false;
/*
* sim
[OSS-Tools] [PATCH v2 1/8] state: backend: direct: open block device in read-only mode if possible
We unconditionally open the device backing a direct bucket in read-write
mode. The barebox_state utility already populates struct
state_backend_storage::readonly though, which we could consult at device
open time. Do so. This could possibly be done for MTD as well, but until
that's tested, for now, we do it only for the direct backend meant for
use with block devices.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/backend_bucket_direct.c | 5 +++--
src/barebox-state/backend_storage.c | 2 +-
src/barebox-state/state.c | 6 +++---
src/barebox-state/state.h | 3 ++-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state/backend_bucket_direct.c
b/src/barebox-state/backend_bucket_direct.c
index 4522f0170f3d..48c1596c9add 100644
--- a/src/barebox-state/backend_bucket_direct.c
+++ b/src/barebox-state/backend_bucket_direct.c
@@ -164,12 +164,13 @@ static void state_backend_bucket_direct_free(struct
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size)
+ off_t offset, ssize_t max_size,
+ bool readonly)
{
int fd;
struct state_backend_storage_bucket_direct *direct;
- fd = open(path, O_RDWR);
+ fd = open(path, readonly ? O_RDONLY : O_RDWR);
if (fd < 0) {
dev_err(dev, "Failed to open file '%s', %d\n", path, -errno);
return -errno;
diff --git a/src/barebox-state/backend_storage.c
b/src/barebox-state/backend_storage.c
index 458f2a91552b..4c5e1783c199 100644
--- a/src/barebox-state/backend_storage.c
+++ b/src/barebox-state/backend_storage.c
@@ -326,7 +326,7 @@ static int state_storage_file_buckets_init(struct
state_backend_storage *storage
offset = storage->offset + n * stridesize;
ret = state_backend_bucket_direct_create(storage->dev,
storage->path,
, offset,
-stridesize);
+stridesize,
storage->readonly);
if (ret) {
dev_warn(storage->dev, "Failed to create direct bucket
at '%s' offset %lld\n",
storage->path, (long long) offset);
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 4779574d2ee9..3174c84b8ded 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -649,14 +649,14 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
if (ret)
goto out_release_state;
+ if (readonly)
+ state_backend_set_readonly(state);
+
ret = state_storage_init(state, state->backend_path, offset,
size, stridesize, storage_type);
if (ret)
goto out_release_state;
- if (readonly)
- state_backend_set_readonly(state);
-
ret = state_from_node(state, node, 1);
if (ret) {
goto out_release_state;
diff --git a/src/barebox-state/state.h b/src/barebox-state/state.h
index 719f7e43c94c..4abfa84285c3 100644
--- a/src/barebox-state/state.h
+++ b/src/barebox-state/state.h
@@ -225,7 +225,8 @@ void state_backend_set_readonly(struct state *state);
void state_storage_free(struct state_backend_storage *storage);
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size);
+ off_t offset, ssize_t max_size,
+ bool readonly);
int state_storage_write(struct state_backend_storage *storage,
const void * buf, ssize_t len);
int state_storage_read(struct state_backend_storage *storage,
--
2.39.2
Re: [OSS-Tools] [PATCH] libdt: fix of_get_devicepath looking up sibling if device unavailable
On 07.06.23 10:08, Ahmad Fatoum wrote:
> of_get_devicepath code flow is split into two:
>
> A) Either the device tree node in question has a direct udev_device
> associated with it
>
> B) Or we assume it's a partition and lookup udev_device for the parent
> first, before finding a child udev_device or setting a partition
> offset within the parent udev_device.
>
> Since v2017.03.0, we have had a fallthrough from case A into case B:
> If we have a udev_device, but it's neither a EEPROMs, MTDs or block
> device, we just consider it a partition. This is problematic, because
> this may result in us pointing at a very different device:
>
> - backend points at a SD-Card host. Host is enabled, but SD-Card
> is not inserted, so no block device
>
> - case A fails, so it's assumed it's a partition and case B
> uses parent SoC bus to lookup appropriate device
>
> - We fall through into the second device_find_block_device, which
> will take the first matching block device across the SoC. So
> we could end up with the eMMC: a completely different device
> than what was pointed at.
>
> Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robust")
> Signed-off-by: Ahmad Fatoum
Patch applied to next.
> ---
> src/libdt.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/libdt.c b/src/libdt.c
> index e54d7fb5649d..7b99efe5b2de 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2492,9 +2492,11 @@ int of_get_devicepath(struct device_node
> *partition_node, char **devpath, off_t
> }
>
> /*
> - * If we found a device but couldn't classify it above, we fall
> - * through.
> + * If we find a udev_device but couldn't classify it above,
> + * it's an error. Falling through would mean to handle it as a
> + * partition and could lead us to return an arbitrary sibling
> device
>*/
> + return -ENODEV;
> }
>
> /*
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs
Hello Roland,
On 07.06.23 10:33, Roland Hieber wrote:
> On Mon, Jun 05, 2023 at 04:32:36PM +0200, Ahmad Fatoum wrote:
>> Despite the generic name, of_find_device_by_uuid only handled
>> ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
>> but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
>>
>> In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
>> to support all three types of GPT UUIDs.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> This patch is the missing prerequisite for my GPT Type UUID series.
>> ---
>> src/libdt.c | 30 +++---
>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/libdt.c b/src/libdt.c
>> index e54d7fb5649d..2c994c647ac9 100644
>> --- a/src/libdt.c
>> +++ b/src/libdt.c
>> @@ -2405,7 +2405,9 @@ out:
>> return dev;
>> }
>>
>> -static struct udev_device *of_find_device_by_uuid(const char *uuid)
>> +static struct udev_device *of_find_device_by_uuid(struct udev_device
>> *parent,
>> + const char *uuid,
>> + bool type_uuid)
>> {
>> struct udev *udev;
>> struct udev_enumerate *enumerate;
>> @@ -2418,12 +2420,15 @@ static struct udev_device
>> *of_find_device_by_uuid(const char *uuid)
>> }
>>
>> enumerate = udev_enumerate_new(udev);
>> +if (parent)
>> +udev_enumerate_add_match_parent(enumerate, parent);
>> udev_enumerate_add_match_subsystem(enumerate, "block");
>> udev_enumerate_scan_devices(enumerate);
>> devices = udev_enumerate_get_list_entry(enumerate);
>> udev_list_entry_foreach(dev_list_entry, devices) {
>> const char *path, *devtype, *dev_uuid;
>> struct udev_device *device;
>> +const char *property;
>>
>> path = udev_list_entry_get_name(dev_list_entry);
>> device = udev_device_new_from_syspath(udev, path);
>> @@ -2432,16 +2437,19 @@ static struct udev_device
>> *of_find_device_by_uuid(const char *uuid)
>> devtype = udev_device_get_devtype(device);
>> if (!devtype)
>> continue;
>> -if (!strcmp(devtype, "disk")) {
>> -dev_uuid = udev_device_get_property_value(device,
>> "ID_PART_TABLE_UUID");
>> -if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> -return device;
>> -} else if (!strcmp(devtype, "partition")) {
>> -dev_uuid = udev_device_get_property_value(device,
>> "ID_PART_ENTRY_UUID");
>> -if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> -return device;
>> -}
>>
>> +if (type_uuid)
>> +property = "ID_PART_ENTRY_TYPE";
>> +else if (!strcmp(devtype, "disk"))
>> +property = "ID_PART_TABLE_UUID";
>> +else if (!strcmp(devtype, "partition"))
>> +property = "ID_PART_ENTRY_UUID";
>> +else
>> +continue;
>> +
>> +dev_uuid = udev_device_get_property_value(device, property);
>> +if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> +return device;
>
> I wonder if we can't simply add a udev_enumerate_add_match_property
> above to do the filtering for us instead of iterating over the match
> entries ourselves.
I didn't know about udev_enumerate_add_match_property. Are multiple filters
OR'd or AND'd? For the !type_uuid case we need to check one or the other
property.
> But the patch is correct, so:
> Reviewed-by: Roland Hieber
Thanks. I have applied this patch and will keep
udev_enumerate_add_match_property
in mind for future rework.
Cheers,
Ahmad
>
>> }
>> return NULL;
>> }
>> @@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node
>> *partition_node, char **devpath, off_t
>> node->full_name);
>> return -ENODEV;
>> }
>> -dev = of_find_device_by_uuid(uuid);
>> +dev = of_find_device_by_uuid(NULL, uuid, false);
>> if (!dev) {
>> fprintf(stderr, "%s: cannot find device for uuid %s\n",
>> __func__,
>> uuid);
>> --
>> 2.39.2
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH] libdt: fix of_get_devicepath looking up sibling if device unavailable
Hello Uwe,
On 07.06.23 10:55, Uwe Kleine-König wrote:
> Hello Ahmad,
>
> On Wed, Jun 07, 2023 at 10:08:18AM +0200, Ahmad Fatoum wrote:
>> of_get_devicepath code flow is split into two:
>>
>> A) Either the device tree node in question has a direct udev_device
>> associated with it
>>
>> B) Or we assume it's a partition and lookup udev_device for the parent
>> first, before finding a child udev_device or setting a partition
>> offset within the parent udev_device.
>>
>> Since v2017.03.0, we have had a fallthrough from case A into case B:
>> If we have a udev_device, but it's neither a EEPROMs, MTDs or block
>> device, we just consider it a partition. This is problematic, because
>> this may result in us pointing at a very different device:
>>
>> - backend points at a SD-Card host. Host is enabled, but SD-Card
>> is not inserted, so no block device
>>
>> - case A fails, so it's assumed it's a partition and case B
>> uses parent SoC bus to lookup appropriate device
>>
>> - We fall through into the second device_find_block_device, which
>> will take the first matching block device across the SoC. So
>> we could end up with the eMMC: a completely different device
>> than what was pointed at.
>
> So another surprise is that device_find_block_device() recurses to find
> a device when starting on /soc, isn't it? Is this worth addressing?
I don't know what I'd break if I limit iteration depth and I don't know
what else I could do to curtail this..
>
>> Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robust")
>> Signed-off-by: Ahmad Fatoum
>> ---
>> src/libdt.c | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libdt.c b/src/libdt.c
>> index e54d7fb5649d..7b99efe5b2de 100644
>> --- a/src/libdt.c
>> +++ b/src/libdt.c
>> @@ -2492,9 +2492,11 @@ int of_get_devicepath(struct device_node
>> *partition_node, char **devpath, off_t
>> }
>>
>> /*
>> - * If we found a device but couldn't classify it above, we fall
>> - * through.
>> + * If we find a udev_device but couldn't classify it above,
>> + * it's an error. Falling through would mean to handle it as a
>> + * partition and could lead us to return an arbitrary sibling
>> device
>> */
>> +return -ENODEV;
>
> I don't remember the details of 929ed64cb42f any more, but probably I
> didn't have a specific case that were fixed by that commit. Your
> rationale makes sense.
>
> Reviewed-by: Uwe Kleine-König
Thanks,
Ahmad
>
> Best regards
> Uwe
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH] libdt: fix of_get_devicepath looking up sibling if device unavailable
of_get_devicepath code flow is split into two:
A) Either the device tree node in question has a direct udev_device
associated with it
B) Or we assume it's a partition and lookup udev_device for the parent
first, before finding a child udev_device or setting a partition
offset within the parent udev_device.
Since v2017.03.0, we have had a fallthrough from case A into case B:
If we have a udev_device, but it's neither a EEPROMs, MTDs or block
device, we just consider it a partition. This is problematic, because
this may result in us pointing at a very different device:
- backend points at a SD-Card host. Host is enabled, but SD-Card
is not inserted, so no block device
- case A fails, so it's assumed it's a partition and case B
uses parent SoC bus to lookup appropriate device
- We fall through into the second device_find_block_device, which
will take the first matching block device across the SoC. So
we could end up with the eMMC: a completely different device
than what was pointed at.
Fixes: 929ed64cb42f ("of_get_devicepath: make partition finding more robust")
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index e54d7fb5649d..7b99efe5b2de 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2492,9 +2492,11 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
}
/*
-* If we found a device but couldn't classify it above, we fall
-* through.
+* If we find a udev_device but couldn't classify it above,
+* it's an error. Falling through would mean to handle it as a
+* partition and could lead us to return an arbitrary sibling
device
*/
+ return -ENODEV;
}
/*
--
2.39.2
[OSS-Tools] [PATCH] libdt: generalize of_find_device_by_uuid for scoped lookup of all UUIDs
Despite the generic name, of_find_device_by_uuid only handled
ID_PART_TABLE_UUID (diskuuid) and ID_PART_ENTRY_UUID (partuuid),
but not ID_PART_ENTRY_TYPE (GPT Partition Type UUID).
In preparation for doing Type UUID lookups, adjust of_find_device_by_uuid
to support all three types of GPT UUIDs.
Signed-off-by: Ahmad Fatoum
---
This patch is the missing prerequisite for my GPT Type UUID series.
---
src/libdt.c | 30 +++---
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index e54d7fb5649d..2c994c647ac9 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2405,7 +2405,9 @@ out:
return dev;
}
-static struct udev_device *of_find_device_by_uuid(const char *uuid)
+static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
+ const char *uuid,
+ bool type_uuid)
{
struct udev *udev;
struct udev_enumerate *enumerate;
@@ -2418,12 +2420,15 @@ static struct udev_device *of_find_device_by_uuid(const
char *uuid)
}
enumerate = udev_enumerate_new(udev);
+ if (parent)
+ udev_enumerate_add_match_parent(enumerate, parent);
udev_enumerate_add_match_subsystem(enumerate, "block");
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
udev_list_entry_foreach(dev_list_entry, devices) {
const char *path, *devtype, *dev_uuid;
struct udev_device *device;
+ const char *property;
path = udev_list_entry_get_name(dev_list_entry);
device = udev_device_new_from_syspath(udev, path);
@@ -2432,16 +2437,19 @@ static struct udev_device *of_find_device_by_uuid(const
char *uuid)
devtype = udev_device_get_devtype(device);
if (!devtype)
continue;
- if (!strcmp(devtype, "disk")) {
- dev_uuid = udev_device_get_property_value(device,
"ID_PART_TABLE_UUID");
- if (dev_uuid && !strcasecmp(dev_uuid, uuid))
- return device;
- } else if (!strcmp(devtype, "partition")) {
- dev_uuid = udev_device_get_property_value(device,
"ID_PART_ENTRY_UUID");
- if (dev_uuid && !strcasecmp(dev_uuid, uuid))
- return device;
- }
+ if (type_uuid)
+ property = "ID_PART_ENTRY_TYPE";
+ else if (!strcmp(devtype, "disk"))
+ property = "ID_PART_TABLE_UUID";
+ else if (!strcmp(devtype, "partition"))
+ property = "ID_PART_ENTRY_UUID";
+ else
+ continue;
+
+ dev_uuid = udev_device_get_property_value(device, property);
+ if (dev_uuid && !strcasecmp(dev_uuid, uuid))
+ return device;
}
return NULL;
}
@@ -2540,7 +2548,7 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
node->full_name);
return -ENODEV;
}
- dev = of_find_device_by_uuid(uuid);
+ dev = of_find_device_by_uuid(NULL, uuid, false);
if (!dev) {
fprintf(stderr, "%s: cannot find device for uuid %s\n",
__func__,
uuid);
--
2.39.2
Re: [OSS-Tools] [PATCH dt-utils 00/14] Sync Barebox-State code base
Hello Marco, On 14.10.22 18:41, Marco Felsch wrote: > Hi, > > this series sync the dt-utils barebox-state code base with a very recent > barebox version [1]. The most import patch are patch13-14, since those adding > the user-space support for the backend format. By this new > backend-format we are able to store the barebox-state on a on-disk > partition like MBR/GPT. > > [1] > https://lore.barebox.org/barebox/20221014163534.3812272-1-m.fel...@pengutronix.de/T/#mb63a3ce9abdfc3091fff9d977e696232713fc247 I applied patches 1-12/14 to next. For the other two, I implemented an alternative scheme that uses GPT Type UUIDs instead: https://lore.pengutronix.org/oss-tools/20230531152253.1407395-1-a.fat...@pengutronix.de/T/#t https://lore.barebox.org/barebox/20230531145927.1399282-1-a.fat...@pengutronix.de/T/#t Thanks, Ahmad > > Regards, > Marco > > Juergen Borleis (1): > libdt: add partition search function > > Marco Felsch (13): > state: Remove duplicate incudes > state: backend_raw: fix ignoring unpack failures > state: backend_storage: deal gracefully with runtime bucket corruption > state: treat state with all-invalid buckets as dirty > state: remove param member from struct state_string > state: remove param member from state_uint32, state_enum32, state_mac > state: remove unused function > state: propagate failure to fixup enum32 into DT > state: add SPDX-License-Identifier for files without explicit license > state: fix typos found with codespell > common: xstrdup: don't panic on xstrdup(NULL) > libdt: add of_property_write_strings support > state: sync with barebox to support new backend type > > src/barebox-state/backend_format_raw.c | 6 +- > src/barebox-state/backend_storage.c| 2 + > src/barebox-state/state.c | 177 +++- > src/barebox-state/state.h | 12 +- > src/barebox-state/state_variables.c| 38 ++-- > src/dt/common.h| 23 ++- > src/dt/dt.h| 4 + > src/libdt-utils.sym| 2 + > src/libdt.c| 271 + > 9 files changed, 452 insertions(+), 83 deletions(-) > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system
On 11.05.22 10:21, Michael Olbrich wrote:
> Hi,
>
> so the discussion on the barebox ML resulted in a different binding for
> this. Sascha has sent patches for that[1]. This is now mainline in Barebox.
>
> So while this is 'v4' for this topic, all the patches except the last one
> are actually different, so please drop the old series that is still present
> in the next branch.
>
> v2 had some Bugs that have been fixed in v3.
> Added some improvements as suggested by Ahmad in v4.
>
> In the device-tree it now looks like this:
Thanks, applied to next branch with a small fixup as mentioned.
>
> --
> / {
> [...]
> state: state {
> [...]
> backend = <_state>;
> [...]
> };
>
> disk {
> compatible = "barebox,storage-by-uuid";
> uuid = "deadbeaf";
>
> partitions {
> compatible = "fixed-partitions";
> #address-cells = <2>;
> #size-cells = <2>;
>
> barebox_state: state@30 {
> label = "barebox-state";
> reg = <0x0 0x30 0x0 0x10>;
> };
> };
> };
> };
> --
>
> Regards,
> Michael
>
> [1]
> https://lore.barebox.org/barebox/20220207094953.949868-1-s.ha...@pengutronix.de/T/#t
>
> Michael Olbrich (3):
> libdt: only requires a partname for mtd
> libdt: add support for barebox,storage-by-uuid
> state: automatically find state.dtb in the ESP
>
> src/barebox-state.c | 25 +
> src/libdt.c | 91 ++---
> 2 files changed, 103 insertions(+), 13 deletions(-)
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables
On 05.06.23 11:30, Ahmad Fatoum wrote:
>> I'm not completely sure which version of the code you are referencing
>> here because I cannot find that context, but …
>
> I replied on mol's series with two fixups that should be squashed. One of them
> removes *outdir, which is written, but never read.
Sorry, I was confused. Please find attached a patch that actually applies on v4.
The other fixup was out-of-place.
-- 8< -
diff --git a/src/libdt.c b/src/libdt.c
index de8209ce53ae..019a7555a6f6 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2363,7 +2363,6 @@ static struct udev_device *of_find_device_by_uuid(const
char *uuid)
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- int ret = 0;
udev = udev_new();
if (!udev) {
@@ -2376,7 +2375,7 @@ static struct udev_device *of_find_device_by_uuid(const
char *uuid)
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
udev_list_entry_foreach(dev_list_entry, devices) {
- const char *path, *devtype, *outpath, *dev_uuid;
+ const char *path, *devtype, *dev_uuid;
struct udev_device *device;
path = udev_list_entry_get_name(dev_list_entry);
@@ -2388,16 +2387,12 @@ static struct udev_device *of_find_device_by_uuid(const
char *uuid)
continue;
if (!strcmp(devtype, "disk")) {
dev_uuid = udev_device_get_property_value(device,
"ID_PART_TABLE_UUID");
- if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
- outpath = udev_device_get_devnode(device);
+ if (dev_uuid && !strcasecmp(dev_uuid, uuid))
return device;
- }
} else if (!strcmp(devtype, "partition")) {
dev_uuid = udev_device_get_property_value(device,
"ID_PART_ENTRY_UUID");
- if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
- outpath = udev_device_get_devnode(device);
+ if (dev_uuid && !strcasecmp(dev_uuid, uuid))
return device;
- }
}
}
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables
On 05.06.23 11:15, Roland Hieber wrote:
> On Wed, May 31, 2023 at 05:13:32PM +0200, Ahmad Fatoum wrote:
>> These variables were recently added, but are unused. Drop them.
>>
>> Signed-off-by: Ahmad Fatoum
>> ---
>> src/libdt.c | 7 ++-
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/libdt.c b/src/libdt.c
>> index 44491a5e739b..302ca7a76375 100644
>> --- a/src/libdt.c
>> +++ b/src/libdt.c
>> @@ -2365,7 +2365,6 @@ static struct udev_device
>> *of_find_device_by_uuid(struct udev_device *parent,
>> struct udev *udev;
>> struct udev_enumerate *enumerate;
>> struct udev_list_entry *devices, *dev_list_entry;
>> -int ret = 0;
>>
>> udev = udev_new();
>> if (!udev) {
>> @@ -2380,7 +2379,7 @@ static struct udev_device
>> *of_find_device_by_uuid(struct udev_device *parent,
>> udev_enumerate_scan_devices(enumerate);
>> devices = udev_enumerate_get_list_entry(enumerate);
>> udev_list_entry_foreach(dev_list_entry, devices) {
>> -const char *path, *devtype, *outpath, *dev_uuid;
>> +const char *path, *devtype, *dev_uuid;
>> struct udev_device *device;
>> const char *property;
>>
>> @@ -2400,10 +2399,8 @@ static struct udev_device
>> *of_find_device_by_uuid(struct udev_device *parent,
>> property = "ID_PART_ENTRY_UUID";
>
> I'm not completely sure which version of the code you are referencing
> here because I cannot find that context, but …
I replied on mol's series with two fixups that should be squashed. One of them
removes *outdir, which is written, but never read.
>
>> dev_uuid = udev_device_get_property_value(device, property);
>> -if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
>> -outpath = udev_device_get_devnode(device);
>
> in mol's v4 patch there's another "outpath" occurrence in the
> 'if (!strcmp(devtype, "disk"))' block above that.
>
> - Roland
>
>> +if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>> return device;
>> -}
>> }
>> return NULL;
>> }
>> --
>> 2.39.2
>>
>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH 4/5] libdt: add CONFIG_TEST_LOOPBACK
We have quite a bit of tricky udev and device tree parsing code that is
only tested manually. Best case we would have tests for this in QEMU,
but until we do, let's test it with loop devices:
The downside is that we can only test block devices and that we need
a tiny bit of code that's not used normally, but on the upside, we
have tests. :-)
Signed-off-by: Ahmad Fatoum
---
configure.ac | 2 ++
meson.build | 1 +
src/dt/dt.h | 1 -
src/libdt.c | 50 +++---
4 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index 117a1e169ba9..5b5c74c2b582 100644
--- a/configure.ac
+++ b/configure.ac
@@ -40,6 +40,8 @@ AC_DEFINE(CONFIG_MTD, [1], [Statically define to be enabled
to harmonize barebox
AC_DEFINE(CONFIG_STATE, [1], [Statically define to be enabled to harmonize
barebox' & dt-utils' code base.])
+AC_DEFINE(CONFIG_TEST_LOOPBACK, [0], [Only enabled in meson for testing.])
+
AC_CHECK_FUNCS([__secure_getenv secure_getenv])
my_CFLAGS="-Wall \
diff --git a/meson.build b/meson.build
index 1bc32274af07..be92446f137a 100644
--- a/meson.build
+++ b/meson.build
@@ -21,6 +21,7 @@ conf.set10('CONFIG_MTD', true)
conf.set10('CONFIG_STATE', true)
conf.set10('CONFIG_STATE_BACKWARD_COMPATIBLE',
get_option('state-backward-compatibility'))
conf.set10('CONFIG_LOCK_DEVICE_NODE', get_option('lock-device'))
+conf.set10('CONFIG_TEST_LOOPBACK', get_option('tests'))
meson.add_dist_script(
find_program('check-news.sh').path(),
diff --git a/src/dt/dt.h b/src/dt/dt.h
index a4213d49606a..153b56e09a21 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -220,7 +220,6 @@ extern int of_set_root_node(struct device_node *node);
extern int of_platform_populate(struct device_node *root,
const struct of_device_id *matches,
struct device_d *parent);
-extern struct device_d *of_find_device_by_node(struct device_node *np);
int of_device_is_stdout_path(struct device_d *dev);
const char *of_get_model(void);
diff --git a/src/libdt.c b/src/libdt.c
index af28de6f17c6..7c24c9197c7f 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2091,6 +2091,22 @@ struct udev_of_path {
static LIST_HEAD(udev_of_devices);
+static const char *udev_device_get_of_path(struct udev_device *dev)
+{
+ const char *of_path;
+
+ of_path = udev_device_get_property_value(dev, "OF_FULLNAME");
+ if (of_path)
+ return of_path;
+
+ if (IS_ENABLED(CONFIG_TEST_LOOPBACK) &&
+ !strcmp(udev_device_get_subsystem(dev), "block") &&
+ !strncmp(udev_device_get_sysname(dev), "loop", 4))
+ return udev_device_get_sysname(dev);
+
+ return NULL;
+}
+
static void of_scan_udev_devices(void)
{
struct udev *udev;
@@ -2111,6 +2127,8 @@ static void of_scan_udev_devices(void)
udev_enumerate_add_match_subsystem(enumerate, "spi");
udev_enumerate_add_match_subsystem(enumerate, "mtd");
udev_enumerate_add_match_subsystem(enumerate, "amba");
+ if (IS_ENABLED(CONFIG_TEST_LOOPBACK))
+ udev_enumerate_add_match_subsystem(enumerate, "block");
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
@@ -2124,7 +2142,7 @@ static void of_scan_udev_devices(void)
path = udev_list_entry_get_name(dev_list_entry);
dev = udev_device_new_from_syspath(udev, path);
- of_path = udev_device_get_property_value(dev, "OF_FULLNAME");
+ of_path = udev_device_get_of_path(dev);
if (!of_path)
continue;
@@ -2148,11 +2166,37 @@ struct udev_device *of_find_device_by_node_path(const
char *of_full_path)
ret = udev_of_path->udev;
break;
}
+ if (!strcmp(udev_of_path->of_path, of_full_path)) {
+ ret = udev_of_path->udev;
+ break;
+ }
}
return ret;
}
+static struct udev_device *of_find_device_by_node(struct device_node *np)
+{
+ struct udev_of_path *udev_of_path;
+ struct udev_device *dev;
+ const char *filename;
+
+ dev = of_find_device_by_node_path(np->full_name);
+ if (dev)
+ return dev;
+
+ if (IS_ENABLED(CONFIG_TEST_LOOPBACK) &&
+ !of_property_read_string(np, "barebox,filename", ) &&
+ !strncmp(filename, "/dev/", 5)) {
+ list_for_each_entry(udev_of_path, _of_devices, list) {
+ if (!strcmp(udev_of_path->of_path, filename + 5))
+ return udev_of_path->udev;
+ }
+ }
+
+ return NULL;
+}
+
static struct udev_dev
[OSS-Tools] [PATCH 2/5] state: add option to lock device node
Even if /run exists, it may not be world-writable. This is the case on
NixOS. Add an alternative option to lock the device node instead.
This should eventually be made the default when it has enjoyed more
testing.
Signed-off-by: Ahmad Fatoum
---
configure.ac | 9 +
meson.build | 1 +
src/barebox-state.c | 30 ++
src/barebox-state/state.c | 4
src/barebox-state/state.h | 21 +
5 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index be8967eb0809..117a1e169ba9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,15 @@ AS_IF([test "x${enable_state_backward_compatibility}" =
"xyes"], [
AC_DEFINE(CONFIG_STATE_BACKWARD_COMPATIBLE, [0])
])
+AC_ARG_ENABLE([lock-device],
+AS_HELP_STRING([--enable-lock-device], [barebox-state: lock device
node instead of global lock in /run @<:@default=disabled@:>@]),
+[], [enable_lock_device=no])
+AS_IF([test "x${enable_lock_device}" = "xyes"], [
+AC_DEFINE(CONFIG_LOCK_DEVICE_NODE, [1], [lock device node backing
state.])
+], [
+AC_DEFINE(CONFIG_LOCK_DEVICE_NODE, [0], [use global lock in /run.])
+])
+
AC_DEFINE(CONFIG_MTD, [1], [Statically define to be enabled to harmonize
barebox' & dt-utils' code base.])
AC_DEFINE(CONFIG_STATE, [1], [Statically define to be enabled to harmonize
barebox' & dt-utils' code base.])
diff --git a/meson.build b/meson.build
index 22b522ea4d0e..2fc13f55ed62 100644
--- a/meson.build
+++ b/meson.build
@@ -20,6 +20,7 @@ conf = configuration_data()
conf.set10('CONFIG_MTD', true)
conf.set10('CONFIG_STATE', true)
conf.set10('CONFIG_STATE_BACKWARD_COMPATIBLE',
get_option('state-backward-compatibility'))
+conf.set10('CONFIG_LOCK_DEVICE_NODE', get_option('lock-device'))
meson.add_dist_script(
find_program('check-news.sh').path(),
diff --git a/src/barebox-state.c b/src/barebox-state.c
index c5acd1f7780a..7cf2fbf070a9 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -499,6 +499,8 @@ int main(int argc, char *argv[])
printf(PACKAGE_STRING "\n");
printf("Configured with build-time option
'--%s-state-backward-compatibility'.\n",
(CONFIG_STATE_BACKWARD_COMPATIBLE) ? "enable" :
"disable");
+ printf("
'--%s-lock-device-node'.\n",
+ (CONFIG_LOCK_DEVICE_NODE) ? "enable" :
"disable");
exit(0);
case 'g':
sg = xzalloc(sizeof(*sg));
@@ -568,17 +570,19 @@ int main(int argc, char *argv[])
++nr_states;
}
- lock_fd = open(BAREBOX_STATE_LOCKFILE, O_CREAT | O_RDWR, 0600);
- if (lock_fd < 0) {
- pr_err("Failed to open lock-file " BAREBOX_STATE_LOCKFILE "\n");
- exit(1);
- }
+ if (!IS_ENABLED(CONFIG_LOCK_DEVICE_NODE)) {
+ lock_fd = open(BAREBOX_STATE_LOCKFILE, O_CREAT | O_RDWR, 0600);
+ if (lock_fd < 0) {
+ pr_err("Failed to open lock-file "
BAREBOX_STATE_LOCKFILE "\n");
+ exit(1);
+ }
- ret = flock(lock_fd, LOCK_EX);
- if (ret < 0) {
- pr_err("Failed to lock " BAREBOX_STATE_LOCKFILE ": %m\n");
- close(lock_fd);
- exit(1);
+ ret = flock(lock_fd, LOCK_EX);
+ if (ret < 0) {
+ pr_err("Failed to lock " BAREBOX_STATE_LOCKFILE ":
%m\n");
+ close(lock_fd);
+ exit(1);
+ }
}
list_for_each_entry(state, _list.list, list) {
@@ -700,8 +704,10 @@ int main(int argc, char *argv[])
ret = 0;
out_unlock:
- flock(lock_fd, LOCK_UN);
- close(lock_fd);
+ if (!IS_ENABLED(CONFIG_LOCK_DEVICE_NODE)) {
+ flock(lock_fd, LOCK_UN);
+ close(lock_fd);
+ }
return ret;
}
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 42ce88d3f161..371ae3959d6c 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -664,6 +664,10 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
pr_debug("%s: backend resolved to %s %lld %zu\n", node->full_name,
state->backend_path, (long long)offset, size);
+ /* will be released on program exit */
+ if (IS_ENABLED(CONFIG_LOCK_DEVICE_NODE))
+ (void)open_exclusive(state->backend_path, readonly ? O_RDONLY :
O_RDWR);
+
state->backend_reproducible_name =
of_get_reproducible_na
[OSS-Tools] [PATCH 1/5] Add meson as build system
This adds experimental support for building dt-utils with meson. The
hope is that the alternative meson.build will be easier to maintain.
Signed-off-by: Ahmad Fatoum
---
.gitignore| 1 +
README| 20 ++
check-news.sh | 82
meson.build | 159 ++
meson_options.txt | 25
version-gen | 3 +
version.h.in | 3 +
7 files changed, 293 insertions(+)
create mode 100755 check-news.sh
create mode 100644 meson.build
create mode 100644 meson_options.txt
create mode 100755 version-gen
create mode 100644 version.h.in
diff --git a/.gitignore b/.gitignore
index 5ee5535f544f..34df220d8723 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@ Makefile.in
/stamp-h1
/tags
/TAGS
+/build/
/fdtdump
/src/libdt-utils.pc
diff --git a/README b/README
index 46b4ea9c1508..0b4e96d6002e 100644
--- a/README
+++ b/README
@@ -13,6 +13,26 @@ For questions, feedback, patches, please send a mail to
Note: you must be subscribed to post to this mailing list. You can do so by
sending an empty mail to .
+Building from Sources
+-
+
+Check out the latest git state:
+
+git clone https://github.com/barebox/dt-utils
+cd dt-utils
+
+And then build using autotools:
+
+./autogen.sh
+./configure
+make
+
+There's also experimental support for building with meson.
+The intention is to deprecate autotools eventually in its favor. To build:
+
+meson setup build
+meson compile -C build
+
Contributing
diff --git a/check-news.sh b/check-news.sh
new file mode 100755
index ..e7e8fa6bb95d
--- /dev/null
+++ b/check-news.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+# Copyright (C) 2019 Red Hat, Inc.
+# Author: Bastien Nocera
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
+
+
+# Add to your top-level meson.build to check for an updated NEWS file
+# when doing a "dist" release, similarly to automake's check-news:
+#
https://www.gnu.org/software/automake/manual/html_node/List-of-Automake-options.html
+#
+# Checks NEWS for the version number:
+# meson.add_dist_script(
+# find_program('check-news.sh').path(),
+# '@0@'.format(meson.project_version())
+# )
+#
+# Checks NEWS and data/foo.appdata.xml for the version number:
+# meson.add_dist_script(
+# find_program('check-news.sh').path(),
+# '@0@'.format(meson.project_version()),
+# 'NEWS',
+# 'data/foo.appdata.xml'
+# )
+
+usage()
+{
+ echo "$0 VERSION [FILES...]"
+ exit 1
+}
+
+check_version()
+{
+ VERSION=$1
+ # Look in the first 15 lines for NEWS files, but look
+ # everywhere for other types of files
+ if [ "$2" = "NEWS" ]; then
+ DATA=`sed 15q $SRC_ROOT/"$2"`
+ else
+ DATA=`cat $SRC_ROOT/"$2"`
+ fi
+ case "$DATA" in
+ *"$VERSION"*)
+ :
+ ;;
+ *)
+ echo "$2 not updated; not releasing" 1>&2;
+ exit 1
+ ;;
+ esac
+}
+
+SRC_ROOT=${MESON_DIST_ROOT:-"./"}
+
+if [ $# -lt 1 ] ; then usage ; fi
+
+VERSION=$1
+shift
+
+if [ $# -eq 0 ] ; then
+ check_version $VERSION 'NEWS'
+ exit 0
+fi
+
+for i in $@ ; do
+ check_version $VERSION "$i"
+done
+
+exit 0
diff --git a/meson.build b/meson.build
new file mode 100644
index ..22b522ea4d0e
--- /dev/null
+++ b/meson.build
@@ -0,0 +1,159 @@
+# Copyright 2013-2023 The DT-Utils Authors
+# Homepage: https://git.pengutronix.de/cgit/tools/dt-utils
+
+project(
+ 'dt-utils',
+ 'c',
+ version : '2021.03.0',
+ meson_version : '>=0.51',
+ default_options: [
+'c_std=gnu11',
+'warning_level=2',
+ ],
+ license : 'GPL-2.0-only',
+)
+
+cc = meson.get_compiler('c')
+
+conf = configuration_data()
+# Statically define to be enabled to harmonize barebox' & dt-utils' code base.
+conf.set10('CONFIG_MTD', true)
+conf.set10('CONFIG_STATE', true)
+conf.set10('CONFIG_STATE_BACKWARD_COMPATIBLE',
get_option('state-backward-compatibility'))
+
+meson.add_dist_script(
+ find_program('check-news.sh').path(),
+ '@0@'.format(meson.project_version())
+)
+
+prefixdir = get_option('pre
[OSS-Tools] [PATCH 3/5] meson: add simple integration test
It's straight forward to use meson as test runner, so let's compile a
very simple test application that links against libdt-utils and verifies
crc32 behaves as one would expect.
Signed-off-by: Ahmad Fatoum
---
README | 1 +
meson.build | 2 ++
test/crc32.c | 18 ++
test/meson.build | 26 ++
4 files changed, 47 insertions(+)
create mode 100644 test/crc32.c
create mode 100644 test/meson.build
diff --git a/README b/README
index 0b4e96d6002e..528751d2cb65 100644
--- a/README
+++ b/README
@@ -32,6 +32,7 @@ The intention is to deprecate autotools eventually in its
favor. To build:
meson setup build
meson compile -C build
+meson test -C build # optional
Contributing
diff --git a/meson.build b/meson.build
index 2fc13f55ed62..1bc32274af07 100644
--- a/meson.build
+++ b/meson.build
@@ -158,3 +158,5 @@ executable('dtblint',
dependencies : [versiondep],
link_with : libdt,
install : true)
+
+subdir('test')
diff --git a/test/crc32.c b/test/crc32.c
new file mode 100644
index ..9a99254c8f22
--- /dev/null
+++ b/test/crc32.c
@@ -0,0 +1,18 @@
+#include
+#include
+
+#include
+
+int main(void)
+{
+ const char *str = "Hello, World!";
+ uint32_t checksum;
+
+ checksum = crc32(0, str, strlen(str));
+ assert(checksum == 0xec4ac3d0);
+
+ checksum = crc32_no_comp(0, str, strlen(str));
+ assert(checksum == 0xe33e8552);
+
+ return 0;
+}
diff --git a/test/meson.build b/test/meson.build
new file mode 100644
index ..5e073547d79b
--- /dev/null
+++ b/test/meson.build
@@ -0,0 +1,26 @@
+if not get_option('tests')
+ subdir_done()
+endif
+
+tests = [
+ 'crc32',
+]
+
+extra_test_sources = files([
+])
+
+foreach test_name : tests
+ exe = executable(
+test_name + '-test',
+test_name + '.c',
+extra_test_sources,
+link_with : [libdt],
+include_directories : incdir)
+
+ test(
+test_name,
+exe,
+is_parallel : false,
+timeout : 240,
+workdir : meson.source_root())
+endforeach
--
2.39.2
[OSS-Tools] [PATCH 5/5] test: add barebox-state loop block device tests
There's certainly more unit tests to be had, but the biggest ROI is
probably feeding barebox-state device trees and verifying that they
work. This same use case exists with RAUC too, where sharness (shell
test harness) is used to test the rauc executable. Let's import it from
there and follow the structure of the RAUC tests to do some basic
testing of barebox-state's block device handling.
The basic testing flow is:
- mount loop devices
- compile and supply device tree snippet describing state on
loop device to the barebox-state utility
- barebox-state is called with verbose output to report which
device it uses
- tests check correct device was used and basic state reading
and writing works.
Signed-off-by: Ahmad Fatoum
---
.gitignore| 1 +
test/01-fixed-partition-no-gpt.dts| 34 +
...2-fixed-partition-before-gpt-partition.dts | 34 +
test/03-fixed-partition-is-gpt-partition.dts | 34 +
test/04-gpt-partition-by-partuuid.dts | 31 +
test/05-gpt-partition-by-typeuuid.dts | 23 +
test/06-fixed-partition-by-diskuuid.dts | 33 +
test/07-raw-disk-fail.dts | 18 +
test/08-gpt-disk-no-typeuuid-fail.dts | 18 +
...-partition-overlaps-two-gpt-partitions.dts | 34 +
...-overlaps-two-gpt-partitions-partially.dts | 34 +
...-fixed-partition-part-of-gpt-partition.dts | 34 +
test/barebox-state.dtsi | 53 ++
test/barebox-state.t | 229 +
test/gpt-no-typeuuid.config | 33 +
test/gpt.config | 35 +
test/meson.build | 10 +
test/raw.config | 24 +
test/sharness.sh | 857 ++
19 files changed, 1569 insertions(+)
create mode 100644 test/01-fixed-partition-no-gpt.dts
create mode 100644 test/02-fixed-partition-before-gpt-partition.dts
create mode 100644 test/03-fixed-partition-is-gpt-partition.dts
create mode 100644 test/04-gpt-partition-by-partuuid.dts
create mode 100644 test/05-gpt-partition-by-typeuuid.dts
create mode 100644 test/06-fixed-partition-by-diskuuid.dts
create mode 100644 test/07-raw-disk-fail.dts
create mode 100644 test/08-gpt-disk-no-typeuuid-fail.dts
create mode 100644 test/31-fixed-partition-overlaps-two-gpt-partitions.dts
create mode 100644
test/32-fixed-partition-overlaps-two-gpt-partitions-partially.dts
create mode 100644 test/33-fixed-partition-part-of-gpt-partition.dts
create mode 100644 test/barebox-state.dtsi
create mode 100755 test/barebox-state.t
create mode 100644 test/gpt-no-typeuuid.config
create mode 100644 test/gpt.config
create mode 100644 test/raw.config
create mode 100755 test/sharness.sh
diff --git a/.gitignore b/.gitignore
index 34df220d8723..22060feceab3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -16,6 +16,7 @@ Makefile.in
/tags
/TAGS
/build/
+/test/test-results
/fdtdump
/src/libdt-utils.pc
diff --git a/test/01-fixed-partition-no-gpt.dts
b/test/01-fixed-partition-no-gpt.dts
new file mode 100644
index ..3ed26e3da468
--- /dev/null
+++ b/test/01-fixed-partition-no-gpt.dts
@@ -0,0 +1,34 @@
+/dts-v1/;
+
+#include "barebox-state.dtsi"
+
+/ {
+ expected-dev = __RAW_LOOPDEV__;
+ expected-partno = <0>; /* unpartitioned space */
+ expected-offset = <0x8000>;
+ expected-size = <0x8000>;
+
+ disk: loopfile {
+ compatible = "barebox,hostfile";
+ barebox,filename = __RAW_LOOPDEV__;
+ barebox,blockdev;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ part_state: state@8000 {
+ reg = <0x8000 0x8000>;
+ label = "state";
+ };
+ };
+ };
+};
+
+ {
+ backend = <_state>;
+ backend-type = "raw";
+ backend-stridesize = <0x40>;
+ backend-storage-type = "direct";
+};
diff --git a/test/02-fixed-partition-before-gpt-partition.dts
b/test/02-fixed-partition-before-gpt-partition.dts
new file mode 100644
index ..b64f910a0cad
--- /dev/null
+++ b/test/02-fixed-partition-before-gpt-partition.dts
@@ -0,0 +1,34 @@
+/dts-v1/;
+
+#include "barebox-state.dtsi"
+
+/ {
+ expected-dev = __GPT_LOOPDEV__;
+ expected-partno = <0>; /* unpartitioned space */
+ expected-offset = <0x8000>;
+ expected-size = <0x8000>;
+
+ disk: loopfile {
+ compatible = "barebox,hostfile";
+ barebox,filename = __GPT_LOOPDEV__;
+ barebox,blockdev;
+
+ partitions
[OSS-Tools] [PATCH 0/5] Add meson support and first test suite
The barebox,state binding is quite complex and we have a lot of udev parsing code that can only be exercised by manually running barebox-state on the target. Make development less error prone, by adding tests for the block device bindings. EEPROM and MTD can follow later. Tests are executed by meson as a runner. Patches to teach autotools to do the same are welcome, although I think we should follow RAUC's steps and eventually deprecate autotools once meson is on par. The obvious wart is that we build with -fvisibility=hidden on autotools, but with meson the same visibility option results in linker errors. I have no idea why yet, but that should only make meson-built libdt-utils a bit slower without functional change. Ahmad Fatoum (5): Add meson as build system state: add option to lock device node meson: add simple integration test libdt: add CONFIG_TEST_LOOPBACK test: add barebox-state loop block device tests .gitignore| 2 + README| 21 + check-news.sh | 82 ++ configure.ac | 11 + meson.build | 163 meson_options.txt | 25 + src/barebox-state.c | 30 +- src/barebox-state/state.c | 4 + src/barebox-state/state.h | 21 + src/dt/dt.h | 1 - src/libdt.c | 50 +- test/01-fixed-partition-no-gpt.dts| 34 + ...2-fixed-partition-before-gpt-partition.dts | 34 + test/03-fixed-partition-is-gpt-partition.dts | 34 + test/04-gpt-partition-by-partuuid.dts | 31 + test/05-gpt-partition-by-typeuuid.dts | 23 + test/06-fixed-partition-by-diskuuid.dts | 33 + test/07-raw-disk-fail.dts | 18 + test/08-gpt-disk-no-typeuuid-fail.dts | 18 + ...-partition-overlaps-two-gpt-partitions.dts | 34 + ...-overlaps-two-gpt-partitions-partially.dts | 34 + ...-fixed-partition-part-of-gpt-partition.dts | 34 + test/barebox-state.dtsi | 53 ++ test/barebox-state.t | 229 + test/crc32.c | 18 + test/gpt-no-typeuuid.config | 33 + test/gpt.config | 35 + test/meson.build | 36 + test/raw.config | 24 + test/sharness.sh | 857 ++ version-gen | 3 + version.h.in | 3 + 32 files changed, 2012 insertions(+), 16 deletions(-) create mode 100755 check-news.sh create mode 100644 meson.build create mode 100644 meson_options.txt create mode 100644 test/01-fixed-partition-no-gpt.dts create mode 100644 test/02-fixed-partition-before-gpt-partition.dts create mode 100644 test/03-fixed-partition-is-gpt-partition.dts create mode 100644 test/04-gpt-partition-by-partuuid.dts create mode 100644 test/05-gpt-partition-by-typeuuid.dts create mode 100644 test/06-fixed-partition-by-diskuuid.dts create mode 100644 test/07-raw-disk-fail.dts create mode 100644 test/08-gpt-disk-no-typeuuid-fail.dts create mode 100644 test/31-fixed-partition-overlaps-two-gpt-partitions.dts create mode 100644 test/32-fixed-partition-overlaps-two-gpt-partitions-partially.dts create mode 100644 test/33-fixed-partition-part-of-gpt-partition.dts create mode 100644 test/barebox-state.dtsi create mode 100755 test/barebox-state.t create mode 100644 test/crc32.c create mode 100644 test/gpt-no-typeuuid.config create mode 100644 test/gpt.config create mode 100644 test/meson.build create mode 100644 test/raw.config create mode 100755 test/sharness.sh create mode 100755 version-gen create mode 100644 version.h.in -- 2.39.2
[OSS-Tools] [PATCH 5/8] libdt: use block device partition instead of parent if found
Linux doesn't parse MTD fixed-partitions described in the device tree
for block devices and EEPROMs. For block devices, The dt-utils work around
this by doing the lookup in userspace and then arrive at the parent block
device along with an offset that is passed along. We can do better
though: If there's a partition block device that contains the region
we are interested in, we should just be using that.
This avoids the issue of triggering a reread of the partition table
after writing barebox state because udev is notified that we had
been opening the whole block device writable.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 60 +++--
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 440fcbd32fb4..e90ac5e26273 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2212,23 +2212,29 @@ static int udev_device_parse_sysattr_u64(struct
udev_device *dev, const char *at
return 0;
}
+/* True if A completely contains B */
+static bool region_contains(loff_t starta, loff_t enda,
+ loff_t startb, loff_t endb)
+{
+return starta <= startb && enda >= endb;
+}
+
/*
- * device_find_block_device - extract device path from udev block device
+ * cdev_from_block_device - Populate cdev from udev block device
*
* @dev: the udev_device to extract information from
- * @devpath: returns the devicepath under which the block device is
accessible
+ * @cdev: the cdev output parameter to populate
*
* returns 0 for success or negative error value on failure.
*/
-int device_find_block_device(struct udev_device *dev,
- char **devpath)
+static int cdev_from_block_device(struct udev_device *dev,
+ struct cdev *cdev)
{
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- struct udev_device *part;
- const char *outpath;
+ struct udev_device *part, *best_match = NULL;
int ret;
udev = udev_new();
@@ -2252,19 +2258,43 @@ int device_find_block_device(struct udev_device *dev,
if (!devtype)
continue;
if (!strcmp(devtype, "disk")) {
- outpath = udev_device_get_devnode(part);
- *devpath = strdup(outpath);
- ret = 0;
- goto out;
+ best_match = part;
+
+ /* Should we try to find a matching partition first? */
+ if (!cdev->size)
+ break;
+ } else if (cdev->size && !strcmp(devtype, "partition")) {
+ u64 partstart, partsize;
+
+ ret = udev_device_parse_sysattr_u64(part, "start",
);
+ if (ret)
+ continue;
+
+ ret = udev_device_parse_sysattr_u64(part, "size",
);
+ if (ret)
+ continue;
+
+ /* start/size sys attributes are always in 512-byte
units */
+ partstart *= 512;
+ partsize *= 512;
+
+ if (!region_contains(partstart, partstart + partsize,
+cdev->offset, cdev->offset +
cdev->size))
+ continue;
+
+ best_match = part;
+ cdev->offset -= partstart;
+ break;
}
}
- ret = -ENODEV;
-out:
+ if (best_match)
+ cdev->devpath = strdup(udev_device_get_devnode(best_match));
+
udev_enumerate_unref(enumerate);
udev_unref(udev);
- return ret;
+ return best_match ? 0 : -ENODEV;
}
/*
@@ -2604,10 +2634,10 @@ static int __of_cdev_find(struct device_node
*partition_node, struct cdev *cdev)
return of_parse_partition(partition_node, >offset,
>size);
} else {
- ret = device_find_block_device(dev, >devpath);
+ ret = of_parse_partition(partition_node, >offset,
>size);
if (ret)
return ret;
- return of_parse_partition(partition_node, >offset,
>size);
+ return cdev_from_block_device(dev, cdev);
}
return -EINVAL;
--
2.39.2
[OSS-Tools] [PATCH 6/8] state: align with barebox use of of_cdev_find
As part of barebox-state by GPT partition type GUID lookup support,
state code in barebox needs direct interaction with the cdev, so it can
enumerate the child partitions. We have recently added __of_cdev_find
with internal linkage, so lets export of_cdev_find with external linkage
for outside use. Unlike __of_cdev_find, the new external function will
return dynamically allocated memory to avoid consumer code having to
know the struct cdev layout.
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/state.c | 26 +++---
src/dt/common.h | 8
src/dt/dt.h | 3 +++
src/libdt-utils.sym | 2 ++
src/libdt.c | 37 +
5 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 3174c84b8ded..29e04c3d7ea8 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -578,6 +578,20 @@ void state_release(struct state *state)
free(state);
}
+#ifdef __BAREBOX__
+static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
+{
+ /*
+* We only accept partitions exactly mapping the barebox-state,
+* but dt-utils may need to set non-zero values here
+*/
+ *offset = 0;
+ *size = 0;
+
+ return basprintf("/dev/%s", cdev->name);
+}
+#endif
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -594,8 +608,9 @@ struct state *state_new_from_node(struct device_node *node,
bool readonly)
const char *alias;
uint32_t stridesize;
struct device_node *partition_node;
- off_t offset = 0;
- size_t size = 0;
+ struct cdev *cdev;
+ off_t offset;
+ size_t size;
alias = of_alias_get(node);
if (!alias) {
@@ -614,11 +629,8 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
-#ifdef __BAREBOX__
- ret = of_find_path_by_node(partition_node, >backend_path, 0);
-#else
- ret = of_get_devicepath(partition_node, >backend_path, ,
);
-#endif
+ cdev = of_cdev_find(partition_node);
+ ret = PTR_ERR_OR_ZERO(cdev);
if (ret) {
if (ret != -EPROBE_DEFER)
dev_err(>dev, "state failed to parse path to
backend: %s\n",
diff --git a/src/dt/common.h b/src/dt/common.h
index d16f1193fbe3..38dc61cd65fe 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -166,6 +166,14 @@ static inline void *ERR_CAST(const void *ptr)
return (void *) ptr;
}
+static inline int PTR_ERR_OR_ZERO(const void *ptr)
+{
+ if (IS_ERR(ptr))
+ return PTR_ERR(ptr);
+ else
+ return 0;
+}
+
static inline char *barebox_asprintf(const char *fmt, ...) __attribute__
((format(__printf__, 1, 2)));
static inline char *barebox_asprintf(const char *fmt, ...)
{
diff --git a/src/dt/dt.h b/src/dt/dt.h
index d5e49eb32df9..f8bd3e56efed 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -231,6 +231,9 @@ void of_add_memory_bank(struct device_node *node, bool
dump, int r,
int of_get_devicepath(struct device_node *partition_node, char **devnode,
off_t *offset,
size_t *size);
+struct cdev *of_cdev_find(struct device_node *partition_node);
+char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
+
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index a749317fa024..f95c74305fb0 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -34,6 +34,8 @@ global:
of_get_child_by_name;
of_get_child_count;
of_get_devicepath;
+ of_cdev_find;
+ cdev_to_devpath;
of_get_next_available_child;
of_get_parent;
of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index e90ac5e26273..1cfca40f9f79 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2677,3 +2677,40 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
return 0;
}
+
+/*
+ * of_cdev_find - get information how to access device corresponding to a
device_node
+ * @partition_node:The device_node which shall be accessed
+ * @devpath: Returns the devicepath under which the device is
accessible
+ * @offset:Returns the offset in the device
+ * @size: Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition.
+ * For this partition the function returns the device path and the offset
+ * and size in the device. For mtd devices the path will be /dev/mtdx, for
+ * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
+ * For mtd devices the device path returned will be the partition itself.
+ * Since EEPR
[OSS-Tools] [PATCH 2/8] libdt: factor out u64 sysattr parsing into helper
We will need to parse two more sysattrs in a follow-up patch, so factor
this out for reuse. While at it switch to 64-bit strtoull: This may
be more useful for future users and unlike atol doesn't invoke undefined
behavior when parsing fails.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 42 +++---
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 2c994c647ac9..580b0b0ba769 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2179,6 +2179,33 @@ static struct udev_device
*device_find_mtd_partition(struct udev_device *dev,
return NULL;
}
+/*
+ * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit
integer
+ * @dev: the udev_device to extract the attribute from
+ * @attr: name of the attribute to lookup
+ * @outvalue: returns the value parsed out of the attribute
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char
*attr,
+ u64 *outvalue)
+{
+ char *endptr;
+ u64 value;
+ const char *str;
+
+ str = udev_device_get_sysattr_value(dev, attr);
+ if (!str)
+ return -EINVAL;
+
+ value = strtoull(str, , 0);
+ if (!*str || *endptr)
+ return -EINVAL;
+
+ *outvalue = value;
+ return 0;
+}
+
/*
* device_find_block_device - extract device path from udev block device
*
@@ -2305,24 +2332,25 @@ static int udev_device_is_eeprom(struct udev_device
*dev)
* udev_parse_mtd - get information from a mtd udev_device
* @dev: the udev_device to extract information from
* @devpath: returns the devicepath under which the mtd device is accessible
- * @size: returns the size of the mtd device
+ * @outsize: returns the size of the mtd device
*
* returns 0 for success or negative error value on failure. *devpath
* will be valid on success and must be freed after usage.
*/
-static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*size)
+static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*outsize)
{
- const char *sizestr;
const char *outpath;
+ u64 size;
+ int ret;
if (!udev_device_is_mtd(dev))
return -EINVAL;
- sizestr = udev_device_get_sysattr_value(dev, "size");
- if (!sizestr)
- return -EINVAL;
+ ret = udev_device_parse_sysattr_u64(dev, "size", );
+ if (ret)
+ return ret;
- *size = atol(sizestr);
+ *outsize = size;
outpath = udev_device_get_devnode(dev);
if (!outpath)
--
2.39.2
[OSS-Tools] [PATCH 0/8] state: allow lookup of barebox state partition by Type GUID
This implements the binding extension introduced to barebox here: https://lore.barebox.org/barebox/20230531145927.1399282-1-a.fat...@pengutronix.de/T/#t With this, barebox,state backend can optionally point at a device instead of a partition. If this device is GPT-partitioned and has a partition with a specific partition type GUID of 4778ed65-bf42-45fa-9c5b-287a1dc4aab1 It will be taken. This series also fixes an annoying issue of barebox-state triggering udev on every access, because the root block device corresponding to the device tree node was opened r/w. barebox-state will now open the disk read-only if possible and if a partition exists that fits the barebox state location, it will be opened instead. Ahmad Fatoum (8): state: backend: direct: open block device in read-only mode if possible libdt: factor out u64 sysattr parsing into helper libdt: drop broken if-branch libdt: factor out __of_cdev_find helper libdt: use block device partition instead of parent if found state: align with barebox use of of_cdev_find libdt: use of_find_device_by_uuid for partuuid lookup state: allow lookup of barebox state partition by Type GUID src/barebox-state/backend_bucket_direct.c | 5 +- src/barebox-state/backend_storage.c | 2 +- src/barebox-state/state.c | 58 +++- src/barebox-state/state.h | 3 +- src/dt/common.h | 8 + src/dt/dt.h | 8 + src/libdt-utils.sym | 5 + src/libdt.c | 341 ++ src/linux/uuid.h | 24 ++ src/state.h | 4 + 10 files changed, 380 insertions(+), 78 deletions(-) create mode 100644 src/linux/uuid.h -- 2.39.2
[OSS-Tools] [PATCH 8/8] state: allow lookup of barebox state partition by Type GUID
The backend device tree property so far always pointed at a partition.
Let's allow pointing it at GPT storage devices directly and lookup
the correct barebox state partition by the well-known type GUID:
4778ed65-bf42-45fa-9c5b-287a1dc4aab1
The implementation is not as neat as hoped for, because lifetime for
barebox cdev and libudev udev_device are quite different and libdt
doesn't yet get the latter right. Once we make sure not to leak
libudev objects and add cdev_unref stubs to barebox, this can be
further simplified by sticking the struct udev_device into struct
cdev and determining extra information on demand.
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/state.c | 26 ++
src/dt/dt.h | 5 ++
src/libdt-utils.sym | 3 ++
src/libdt.c | 101 --
src/linux/uuid.h | 24 +
src/state.h | 4 ++
6 files changed, 160 insertions(+), 3 deletions(-)
create mode 100644 src/linux/uuid.h
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 29e04c3d7ea8..42ce88d3f161 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
@@ -592,6 +593,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t
*offset, size_t *size)
}
#endif
+static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
+
/*
* state_new_from_node - create a new state instance from a device_node
*
@@ -638,6 +641,29 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
goto out_release_state;
}
+ /* Is the backend referencing an on-disk partitonable block device? */
+ if (cdev_is_block_disk(cdev)) {
+ struct cdev *partcdev = NULL;
+
+ if (cdev_is_gpt_partitioned(cdev))
+ partcdev = cdev_find_child_by_typeuuid(cdev,
_state_partition_guid);
+
+ if (!partcdev) {
+ ret = -EINVAL;
+ goto out_release_state;
+ }
+
+ pr_debug("%s: backend GPT partition looked up via
PartitionTypeGUID\n",
+node->full_name);
+
+ cdev = partcdev;
+ }
+
+ state->backend_path = cdev_to_devpath(cdev, , );
+
+ pr_debug("%s: backend resolved to %s %lld %zu\n", node->full_name,
+state->backend_path, (long long)offset, size);
+
state->backend_reproducible_name =
of_get_reproducible_name(partition_node);
ret = of_property_read_string(node, "backend-type", _type);
diff --git a/src/dt/dt.h b/src/dt/dt.h
index f8bd3e56efed..a4213d49606a 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
/* Default string compare functions */
#define of_compat_cmp(s1, s2, l) strcasecmp((s1), (s2))
@@ -234,6 +235,10 @@ int of_get_devicepath(struct device_node *partition_node,
char **devnode, off_t
struct cdev *of_cdev_find(struct device_node *partition_node);
char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
+bool cdev_is_block_disk(struct cdev *cdev);
+bool cdev_is_gpt_partitioned(struct cdev *cdev);
+struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev, guid_t *typeuuid);
+
#define for_each_node_by_name(dn, name) \
for (dn = of_find_node_by_name(NULL, name); dn; \
dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index f95c74305fb0..63536224e518 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -36,6 +36,9 @@ global:
of_get_devicepath;
of_cdev_find;
cdev_to_devpath;
+ cdev_is_block_disk;
+ cdev_is_gpt_partitioned;
+ cdev_find_child_by_typeuuid;
of_get_next_available_child;
of_get_parent;
of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index 4c000919d305..af28de6f17c6 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -30,12 +30,15 @@
#include
#include
#include
+#include
#include
struct cdev {
char *devpath;
off_t offset;
size_t size;
+ bool is_gpt_partitioned;
+ bool is_block_disk;
};
static int pr_level = 5;
@@ -2230,7 +2233,7 @@ static bool region_contains(loff_t starta, loff_t enda,
static int cdev_from_block_device(struct udev_device *dev,
struct cdev *cdev)
{
-
+ const char *devtype;
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
@@ -2250,7 +2253,7 @@ static int cdev_from_block_device(struct udev_device *dev,
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
udev_list_entry_foreach(dev_list_entry, devices) {
- const cha
[OSS-Tools] [PATCH 4/8] libdt: factor out __of_cdev_find helper
of_get_devicepath is an exported symbol by libdt, so we shouldn't change
its signature or its semantics. Yet, we will want to export more
information out of the udev'ification code in future. already
declares an opaque struct cdev, so let's add a __of_cdev_find helper
that can populate it with the same information that of_get_devicepath
would return. In later commits we will define helpers that return
and process a struct cdev placed in dynamic memory.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 82 +
1 file changed, 52 insertions(+), 30 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 12d692d2b2cf..440fcbd32fb4 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -32,6 +32,12 @@
#include
#include
+struct cdev {
+ char *devpath;
+ off_t offset;
+ size_t size;
+};
+
static int pr_level = 5;
void pr_level_set(int level)
@@ -2482,33 +2488,14 @@ static struct udev_device
*of_find_device_by_uuid(struct udev_device *parent,
return NULL;
}
-/*
- * of_get_devicepath - get information how to access device corresponding to a
device_node
- * @partition_node:The device_node which shall be accessed
- * @devpath: Returns the devicepath under which the device is
accessible
- * @offset:Returns the offset in the device
- * @size: Returns the size of the device
- *
- * This function takes a device_node which represents a partition.
- * For this partition the function returns the device path and the offset
- * and size in the device. For mtd devices the path will be /dev/mtdx, for
- * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
- * For mtd devices the device path returned will be the partition itself.
- * Since EEPROMs do not have partitions under Linux @offset and @size will
- * describe the offset and size inside the full device. The same applies to
- * block devices.
- *
- * returns 0 for success or negative error value on failure.
- */
-int of_get_devicepath(struct device_node *partition_node, char **devpath,
off_t *offset,
- size_t *size)
+static int __of_cdev_find(struct device_node *partition_node, struct cdev
*cdev)
{
struct device_node *node;
struct udev_device *dev, *partdev, *mtd;
int ret;
- *offset = 0;
- *size = 0;
+ cdev->offset = 0;
+ cdev->size = 0;
/*
* simplest case: This nodepath can directly be translated into
@@ -2520,8 +2507,8 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
dev = of_find_device_by_node_path(partition_node->full_name);
if (dev) {
if (udev_device_is_eeprom(dev))
- return udev_parse_eeprom(dev, devpath);
- if (!udev_parse_mtd(dev, devpath, size))
+ return udev_parse_eeprom(dev, >devpath);
+ if (!udev_parse_mtd(dev, >devpath, >size))
return 0;
/*
@@ -2551,7 +2538,7 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
while (*uuid)
*s++ = tolower(*uuid++);
- *devpath = lc_uuid;
+ cdev->devpath = lc_uuid;
return 0;
}
@@ -2607,21 +2594,56 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
return -ENODEV;
/* ...find the desired information by mtd udev_device */
- return udev_parse_mtd(partdev, devpath, size);
+ return udev_parse_mtd(partdev, >devpath, >size);
}
if (udev_device_is_eeprom(dev)) {
- ret = udev_parse_eeprom(dev, devpath);
+ ret = udev_parse_eeprom(dev, >devpath);
if (ret)
return ret;
- return of_parse_partition(partition_node, offset, size);
+ return of_parse_partition(partition_node, >offset,
>size);
} else {
- ret = device_find_block_device(dev, devpath);
+ ret = device_find_block_device(dev, >devpath);
if (ret)
return ret;
- return of_parse_partition(partition_node, offset, size);
+ return of_parse_partition(partition_node, >offset,
>size);
}
return -EINVAL;
}
+
+/*
+ * of_get_devicepath - get information how to access device corresponding to a
device_node
+ * @partition_node:The device_node which shall be accessed
+ * @devpath: Returns the devicepath under which the device is
accessible
+ * @offset:Returns the offset in the device
+ * @size: Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition
[OSS-Tools] [PATCH 1/8] state: backend: direct: open block device in read-only mode if possible
We unconditionally open the device backing a direct bucket in read-write
mode. The barebox_state utility already populates struct
state_backend_storage::readonly though, which we could consult at device
open time. Do so. This could possibly be done for MTD as well, but until
that's tested, for now, we do it only for the direct backend meant for
use with block devices.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/backend_bucket_direct.c | 5 +++--
src/barebox-state/backend_storage.c | 2 +-
src/barebox-state/state.c | 6 +++---
src/barebox-state/state.h | 3 ++-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state/backend_bucket_direct.c
b/src/barebox-state/backend_bucket_direct.c
index 4522f0170f3d..48c1596c9add 100644
--- a/src/barebox-state/backend_bucket_direct.c
+++ b/src/barebox-state/backend_bucket_direct.c
@@ -164,12 +164,13 @@ static void state_backend_bucket_direct_free(struct
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size)
+ off_t offset, ssize_t max_size,
+ bool readonly)
{
int fd;
struct state_backend_storage_bucket_direct *direct;
- fd = open(path, O_RDWR);
+ fd = open(path, readonly ? O_RDONLY : O_RDWR);
if (fd < 0) {
dev_err(dev, "Failed to open file '%s', %d\n", path, -errno);
return -errno;
diff --git a/src/barebox-state/backend_storage.c
b/src/barebox-state/backend_storage.c
index 458f2a91552b..4c5e1783c199 100644
--- a/src/barebox-state/backend_storage.c
+++ b/src/barebox-state/backend_storage.c
@@ -326,7 +326,7 @@ static int state_storage_file_buckets_init(struct
state_backend_storage *storage
offset = storage->offset + n * stridesize;
ret = state_backend_bucket_direct_create(storage->dev,
storage->path,
, offset,
-stridesize);
+stridesize,
storage->readonly);
if (ret) {
dev_warn(storage->dev, "Failed to create direct bucket
at '%s' offset %lld\n",
storage->path, (long long) offset);
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 4779574d2ee9..3174c84b8ded 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -649,14 +649,14 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
if (ret)
goto out_release_state;
+ if (readonly)
+ state_backend_set_readonly(state);
+
ret = state_storage_init(state, state->backend_path, offset,
size, stridesize, storage_type);
if (ret)
goto out_release_state;
- if (readonly)
- state_backend_set_readonly(state);
-
ret = state_from_node(state, node, 1);
if (ret) {
goto out_release_state;
diff --git a/src/barebox-state/state.h b/src/barebox-state/state.h
index 719f7e43c94c..4abfa84285c3 100644
--- a/src/barebox-state/state.h
+++ b/src/barebox-state/state.h
@@ -225,7 +225,8 @@ void state_backend_set_readonly(struct state *state);
void state_storage_free(struct state_backend_storage *storage);
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size);
+ off_t offset, ssize_t max_size,
+ bool readonly);
int state_storage_write(struct state_backend_storage *storage,
const void * buf, ssize_t len);
int state_storage_read(struct state_backend_storage *storage,
--
2.39.2
[OSS-Tools] [PATCH 7/8] libdt: use of_find_device_by_uuid for partuuid lookup
With the addition of of_find_device_by_uuid as part of the support for
barebox,storage-by-uuid, we don't need to rely on the device symlinks
created by 60-persistent-storage.rules and instead can just use libudev
directly, which we do elsewhere anyway. This has the added benefit
that we unify with the other code paths, where we determine the device
path through the struct udev_device.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 1cfca40f9f79..4c000919d305 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2558,18 +2558,22 @@ static int __of_cdev_find(struct device_node
*partition_node, struct cdev *cdev)
/* when partuuid is specified short-circuit the search for the
cdev */
ret = of_property_read_string(partition_node, "partuuid",
);
if (!ret) {
- const char prefix[] = "/dev/disk/by-partuuid/";
- size_t prefix_len = sizeof(prefix) - 1;
- char *lc_uuid, *s;
+ u64 partsize;
+ int ret;
- lc_uuid = xzalloc(prefix_len + strlen(uuid) + 1);
- s = memcpy(lc_uuid, prefix, prefix_len) + prefix_len;
+ dev = of_find_device_by_uuid(NULL, uuid, false);
+ if (!dev) {
+ fprintf(stderr, "%s: cannot find device for
uuid %s\n", __func__,
+ uuid);
+ return -ENODEV;
+ }
- while (*uuid)
- *s++ = tolower(*uuid++);
-
- cdev->devpath = lc_uuid;
+ ret = udev_device_parse_sysattr_u64(dev, "size",
);
+ if (ret)
+ return -EINVAL;
+ cdev->size = partsize * 512;
+ cdev->devpath = strdup(udev_device_get_devnode(dev));
return 0;
}
}
--
2.39.2
[OSS-Tools] [PATCH 3/8] libdt: drop broken if-branch
device_find_block_device returns 0 on success, so the way the else if
clause is now, only if there is a block device, the code falls through.
If there is none, a 0 is returned, but devpath is not populated breaking
the contract of the function. Just drop the branch for now and add back
it later in a way that works.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 580b0b0ba769..12d692d2b2cf 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2519,13 +2519,10 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
*/
dev = of_find_device_by_node_path(partition_node->full_name);
if (dev) {
- if (udev_device_is_eeprom(dev)) {
+ if (udev_device_is_eeprom(dev))
return udev_parse_eeprom(dev, devpath);
- } else if (!udev_parse_mtd(dev, devpath, size)) {
+ if (!udev_parse_mtd(dev, devpath, size))
return 0;
- } else if (device_find_block_device(dev, devpath)) {
- return of_parse_partition(partition_node, offset, size);
- }
/*
* If we found a device but couldn't classify it above, we fall
--
2.39.2
Re: [OSS-Tools] [PATCH v4 2/3] libdt: add support for barebox, storage-by-uuid
Hello Michael,
On 11.05.22 10:21, Michael Olbrich wrote:
> Signed-off-by: Michael Olbrich
Tested-by: Ahmad Fatoum
I'd squash the two fixups I just sent when I apply this.
Thanks,
Ahmad
> ---
>
> changes in v3:
> uuid comparison fixed:
> - compare the correct uuids
> - don't crash when no uuid is present
>
> changes in v4:
> use strcasecmp() to make the uuid check case insensitive.
>
> src/libdt.c | 74 +
> 1 file changed, 69 insertions(+), 5 deletions(-)
>
> diff --git a/src/libdt.c b/src/libdt.c
> index 48c31931e8a1..4076d9a2f4a3 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2358,6 +2358,52 @@ out:
> return dev;
> }
>
> +static struct udev_device *of_find_device_by_uuid(const char *uuid)
> +{
> + struct udev *udev;
> + struct udev_enumerate *enumerate;
> + struct udev_list_entry *devices, *dev_list_entry;
> + int ret = 0;
> +
> + udev = udev_new();
> + if (!udev) {
> + fprintf(stderr, "Can't create udev\n");
> + return NULL;
> + }
> +
> + enumerate = udev_enumerate_new(udev);
> + udev_enumerate_add_match_subsystem(enumerate, "block");
> + udev_enumerate_scan_devices(enumerate);
> + devices = udev_enumerate_get_list_entry(enumerate);
> + udev_list_entry_foreach(dev_list_entry, devices) {
> + const char *path, *devtype, *outpath, *dev_uuid;
> + struct udev_device *device;
> +
> + path = udev_list_entry_get_name(dev_list_entry);
> + device = udev_device_new_from_syspath(udev, path);
> +
> + /* distinguish device (disk) from partitions */
> + devtype = udev_device_get_devtype(device);
> + if (!devtype)
> + continue;
> + if (!strcmp(devtype, "disk")) {
> + dev_uuid = udev_device_get_property_value(device,
> "ID_PART_TABLE_UUID");
> + if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> + outpath = udev_device_get_devnode(device);
> + return device;
> + }
> + } else if (!strcmp(devtype, "partition")) {
> + dev_uuid = udev_device_get_property_value(device,
> "ID_PART_ENTRY_UUID");
> + if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> + outpath = udev_device_get_devnode(device);
> + return device;
> + }
> + }
> +
> + }
> + return NULL;
> +}
> +
> /*
> * of_get_devicepath - get information how to access device corresponding to
> a device_node
> * @partition_node: The device_node which shall be accessed
> @@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node
> *partition_node, char **devpath, off_t
> if (!strcmp(node->name, "partitions"))
> node = node->parent;
>
> - dev = of_find_device_by_node_path(node->full_name);
> - if (!dev) {
> - fprintf(stderr, "%s: cannot find device from node %s\n",
> __func__,
> - node->full_name);
> - return -ENODEV;
> + if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
> + const char *uuid;
> +
> + ret = of_property_read_string(node, "uuid", );
> + if (ret) {
> + fprintf(stderr, "%s: missing uuid property for %s\n",
> __func__,
> + node->full_name);
> + return -ENODEV;
> + }
> + dev = of_find_device_by_uuid(uuid);
> + if (!dev) {
> + fprintf(stderr, "%s: cannot find device for uuid %s\n",
> __func__,
> + uuid);
> + return -ENODEV;
> + }
> + }
> + else {
> + dev = of_find_device_by_node_path(node->full_name);
> + if (!dev) {
> + fprintf(stderr, "%s: cannot find device from node
> %s\n", __func__,
> + node->full_name);
> + return -ENODEV;
> + }
> }
>
> mtd = of_find_mtd_device(dev);
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[OSS-Tools] [PATCH fixup 2/2] libdt: fix possible use of uninitialized variable
property may not be always defined. So let's skip trying to do something with it in that case. Signed-off-by: Ahmad Fatoum --- src/libdt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libdt.c b/src/libdt.c index 302ca7a76375..ca0502ac9483 100644 --- a/src/libdt.c +++ b/src/libdt.c @@ -2397,6 +2397,8 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent, property = "ID_PART_TABLE_UUID"; else if (!strcmp(devtype, "partition")) property = "ID_PART_ENTRY_UUID"; + else + continue; dev_uuid = udev_device_get_property_value(device, property); if (dev_uuid && !strcasecmp(dev_uuid, uuid)) -- 2.39.2
[OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables
These variables were recently added, but are unused. Drop them.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 44491a5e739b..302ca7a76375 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2365,7 +2365,6 @@ static struct udev_device *of_find_device_by_uuid(struct
udev_device *parent,
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- int ret = 0;
udev = udev_new();
if (!udev) {
@@ -2380,7 +2379,7 @@ static struct udev_device *of_find_device_by_uuid(struct
udev_device *parent,
udev_enumerate_scan_devices(enumerate);
devices = udev_enumerate_get_list_entry(enumerate);
udev_list_entry_foreach(dev_list_entry, devices) {
- const char *path, *devtype, *outpath, *dev_uuid;
+ const char *path, *devtype, *dev_uuid;
struct udev_device *device;
const char *property;
@@ -2400,10 +2399,8 @@ static struct udev_device *of_find_device_by_uuid(struct
udev_device *parent,
property = "ID_PART_ENTRY_UUID";
dev_uuid = udev_device_get_property_value(device, property);
- if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
- outpath = udev_device_get_devnode(device);
+ if (dev_uuid && !strcasecmp(dev_uuid, uuid))
return device;
- }
}
return NULL;
}
--
2.39.2
[OSS-Tools] [PATCH 3/5] libdt: use memcpy instead of strncpy
Despite the name, GCC objects at the strncpy use in safe_strncpy on
safety grounds. While that seems to be a false positive, we could
just be using memcpy instead and side step this altogether.
Signed-off-by: Ahmad Fatoum
---
src/dt/common.h | 34 ++
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/dt/common.h b/src/dt/common.h
index c3c4f53fc216..69a264cfc1a9 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -36,6 +36,12 @@ typedef uint64_t u64;
#undef offsetof
#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER)
+#define min(x, y) ({ \
+ typeof(x) _min1 = (x); \
+ typeof(y) _min2 = (y); \
+ (void) (&_min1 == &_min2); \
+ _min1 < _min2 ? _min1 : _min2; })
+
struct device_d;
void pr_level_set(int level);
@@ -199,14 +205,6 @@ static inline size_t DT_strlcpy(char *dest, const char
*src, size_t size)
return ret;
}
-/* Like strncpy but make sure the resulting string is always 0 terminated. */
-static inline char * safe_strncpy(char *dst, const char *src, size_t size)
-{
- if (!size) return dst;
- dst[--size] = '\0';
- return strncpy(dst, src, size);
-}
-
static inline char *xstrdup(const char *s)
{
char *p = strdup(s);
@@ -415,21 +413,23 @@ static inline int dev_set_name(struct device_d *dev,
const char *fmt, ...)
{
char newname[MAX_DRIVER_NAME];
va_list vargs;
- int err;
+ int ret;
va_start(vargs, fmt);
- err = vsnprintf(newname, MAX_DRIVER_NAME, fmt, vargs);
+ ret = vsnprintf(newname, MAX_DRIVER_NAME, fmt, vargs);
va_end(vargs);
+ if (WARN_ON(ret < 0))
+ return ret;
+
/*
* Copy new name into dev structure, we do this after vsnprintf call in
* case old device name was in one of vargs
*/
- safe_strncpy(dev->name, newname, MAX_DRIVER_NAME);
+ memcpy(dev->name, newname, min(MAX_DRIVER_NAME - 1, ret));
+ dev->name[MAX_DRIVER_NAME - 1] = '\0';
- WARN_ON(err < 0);
-
- return err;
+ return 0;
}
struct driver_d;
@@ -577,12 +577,6 @@ static inline __u32 ror32(__u32 word, unsigned int shift)
return (word >> shift) | (word << (32 - shift));
}
-#define min(x, y) ({ \
- typeof(x) _min1 = (x); \
- typeof(y) _min2 = (y); \
- (void) (&_min1 == &_min2); \
- _min1 < _min2 ? _min1 : _min2; })
-
/*
* Helper macros to use CONFIG_ options in C expressions. Note that
* these only work with boolean and tristate options.
--
2.39.2
[OSS-Tools] [PATCH 4/5] libdt: don't use old-style function definition
Increasing the warning level has GCC warn us, so let's heed the warning.
Signed-off-by: Ahmad Fatoum
---
src/dt/common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/dt/common.h b/src/dt/common.h
index 69a264cfc1a9..62776c0bb027 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -460,7 +460,7 @@ static inline int of_unregister_fixup(int (*fixup)(struct
device_node *, void *)
}
#define __define_initcall(level,fn,id) \
-static void __attribute__ ((constructor)) __initcall_##id##_##fn() { \
+static void __attribute__ ((constructor)) __initcall_##id##_##fn(void) { \
fn(); \
}
--
2.39.2
[OSS-Tools] [PATCH 1/5] configure: pass -fno-strict-aliasing to GCC
Like the Linux kernel, barebox is built with -fno-strict-aliasing, and code is written to take advantage of that. As dt-utils imports code from barebox, we may import code that assumes aliasing to be non-strict, so we better compile it with the same rules. Signed-off-by: Ahmad Fatoum --- configure.ac | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 00c80105e467..be8967eb0809 100644 --- a/configure.ac +++ b/configure.ac @@ -37,7 +37,8 @@ my_CFLAGS="-Wall \ -Wmissing-declarations -Wmissing-prototypes \ -Wnested-externs -Wsign-compare -Wchar-subscripts \ -Wstrict-prototypes -Wshadow \ --Wformat-security -Wtype-limits" +-Wformat-security -Wtype-limits \ +-fno-strict-aliasing" AC_SUBST([my_CFLAGS]) PKG_CHECK_MODULES(UDEV, [libudev]) -- 2.39.2
[OSS-Tools] [PATCH 2/5] libdt: fix issues of external function without prototype
When increasing warning level, we see that of_find_device_by_node_path
lacks a prototype despite having external linkage and being exported
in the symbols file. On the other hand, scan_proc_dir has external
linkage, but is only ever needed internally, so let's give it internal
linkage.
Signed-off-by: Ahmad Fatoum
---
src/crc32.c | 1 +
src/dt/dt.h | 1 +
src/libdt.c | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/crc32.c b/src/crc32.c
index 8d4dddcf6129..8273d3465f6f 100644
--- a/src/crc32.c
+++ b/src/crc32.c
@@ -8,6 +8,7 @@
* For conditions of distribution and use, see copyright notice in zlib.h
*/
+#include
#include
/*
diff --git a/src/dt/dt.h b/src/dt/dt.h
index 4ae24ba8bf7a..6ce95d91da87 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -121,6 +121,7 @@ extern struct device_node *of_find_node_by_type(struct
device_node *from,
const char *type);
extern struct device_node *of_find_compatible_node(struct device_node *from,
const char *type, const char *compat);
+extern struct udev_device *of_find_device_by_node_path(const char
*of_full_path);
extern const struct of_device_id *of_match_node(
const struct of_device_id *matches, const struct device_node *node);
extern struct device_node *of_find_matching_node_and_match(
diff --git a/src/libdt.c b/src/libdt.c
index 59e76d336d8d..a833c582dfbf 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -1938,7 +1938,7 @@ int of_device_disable_path(const char *path)
return of_device_disable(node);
}
-int scan_proc_dir(struct device_node *node, const char *path)
+static int scan_proc_dir(struct device_node *node, const char *path)
{
DIR *dir;
struct dirent *dirent;
--
2.39.2
[OSS-Tools] [PATCH 5/5] barebox-state: fix use after free in error path
blob_bin is freed a few lines above unconditionally, so freeing it
again in the error path will cause a double free.
Signed-off-by: Ahmad Fatoum
---
src/keystore-blob.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/keystore-blob.c b/src/keystore-blob.c
index ed6ecb4eaa25..8ec07f0a3d56 100644
--- a/src/keystore-blob.c
+++ b/src/keystore-blob.c
@@ -81,10 +81,8 @@ int keystore_get_secret(const char *name, const unsigned
char **key, int *key_le
/* payload */
fd = open(blob_gen_payload, O_RDONLY);
- if (fd < 0) {
- free(blob_bin);
+ if (fd < 0)
return -errno;
- }
payload = xzalloc(len);
len = read(fd, payload, len);
--
2.39.2
[OSS-Tools] [PATCH 0/3] state: void partition table reread when possible
So far, barebox-state with a MMC backend always opened the whole disk read/write, which triggers a partition table reread when closing the device. This can be avoided in some cases: - If we are only interested in reading, we can open it O_RDONLY - If there happens to be an on-disk-described partition at exactly the region we are interested, we can open that instead. So let's do that. The result can be easily verified with strace. Without this patch: openat(AT_FDCWD, "/dev/mmcblk0", O_RDWR|O_LARGEFILE) = 4 With this patch: openat(AT_FDCWD, "/dev/mmcblk0p6", O_RDONLY|O_LARGEFILE) = 4 Ahmad Fatoum (3): state: backend: direct: open block device in read-only mode if possible libdt: factor out u64 sysattr parsing into helper libdt: use block device partition instead of parent if found src/barebox-state/backend_bucket_direct.c | 5 +- src/barebox-state/backend_storage.c | 2 +- src/barebox-state/state.c | 6 +- src/barebox-state/state.h | 3 +- src/libdt.c | 104 +- 5 files changed, 92 insertions(+), 28 deletions(-) -- 2.39.2
[OSS-Tools] [PATCH 2/3] libdt: factor out u64 sysattr parsing into helper
We will need to parse two more sysattrs in a follow-up patch, so factor
this out for reuse. While at it switch to 64-bit strtoull: This may
be more useful for future users and unlike atol doesn't invoke undefined
behavior when parsing fails.
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 42 +++---
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 59e76d336d8d..6f1a9e309d53 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2132,6 +2132,33 @@ static struct udev_device
*device_find_mtd_partition(struct udev_device *dev,
return NULL;
}
+/*
+ * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit
integer
+ * @dev: the udev_device to extract the attribute from
+ * @attr: name of the attribute to lookup
+ * @outvalue: returns the value parsed out of the attribute
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char
*attr,
+ u64 *outvalue)
+{
+ char *endptr;
+ u64 value;
+ const char *str;
+
+ str = udev_device_get_sysattr_value(dev, attr);
+ if (!str)
+ return -EINVAL;
+
+ value = strtoull(str, , 0);
+ if (!*str || *endptr)
+ return -EINVAL;
+
+ *outvalue = value;
+ return 0;
+}
+
/*
* device_find_block_device - extract device path from udev block device
*
@@ -2258,24 +2285,25 @@ static int udev_device_is_eeprom(struct udev_device
*dev)
* udev_parse_mtd - get information from a mtd udev_device
* @dev: the udev_device to extract information from
* @devpath: returns the devicepath under which the mtd device is accessible
- * @size: returns the size of the mtd device
+ * @outsize: returns the size of the mtd device
*
* returns 0 for success or negative error value on failure. *devpath
* will be valid on success and must be freed after usage.
*/
-static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*size)
+static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t
*outsize)
{
- const char *sizestr;
const char *outpath;
+ u64 size;
+ int ret;
if (!udev_device_is_mtd(dev))
return -EINVAL;
- sizestr = udev_device_get_sysattr_value(dev, "size");
- if (!sizestr)
- return -EINVAL;
+ ret = udev_device_parse_sysattr_u64(dev, "size", );
+ if (ret)
+ return ret;
- *size = atol(sizestr);
+ *outsize = size;
outpath = udev_device_get_devnode(dev);
if (!outpath)
--
2.39.2
[OSS-Tools] [PATCH 1/3] state: backend: direct: open block device in read-only mode if possible
We unconditionally open the device backing a direct bucket in read-write
mode. The barebox_state utility already populates struct
state_backend_storage::readonly though, which we could consult at device
open time. Do so. This could possibly be done for MTD as well, but until
that's tested, for now, we do it only for the direct backend meant for
use with block devices.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/barebox-state/backend_bucket_direct.c | 5 +++--
src/barebox-state/backend_storage.c | 2 +-
src/barebox-state/state.c | 6 +++---
src/barebox-state/state.h | 3 ++-
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state/backend_bucket_direct.c
b/src/barebox-state/backend_bucket_direct.c
index 4522f0170f3d..48c1596c9add 100644
--- a/src/barebox-state/backend_bucket_direct.c
+++ b/src/barebox-state/backend_bucket_direct.c
@@ -164,12 +164,13 @@ static void state_backend_bucket_direct_free(struct
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size)
+ off_t offset, ssize_t max_size,
+ bool readonly)
{
int fd;
struct state_backend_storage_bucket_direct *direct;
- fd = open(path, O_RDWR);
+ fd = open(path, readonly ? O_RDONLY : O_RDWR);
if (fd < 0) {
dev_err(dev, "Failed to open file '%s', %d\n", path, -errno);
return -errno;
diff --git a/src/barebox-state/backend_storage.c
b/src/barebox-state/backend_storage.c
index 509427f16f1d..39c2b9803f2d 100644
--- a/src/barebox-state/backend_storage.c
+++ b/src/barebox-state/backend_storage.c
@@ -324,7 +324,7 @@ static int state_storage_file_buckets_init(struct
state_backend_storage *storage
offset = storage->offset + n * stridesize;
ret = state_backend_bucket_direct_create(storage->dev,
storage->path,
, offset,
-stridesize);
+stridesize,
storage->readonly);
if (ret) {
dev_warn(storage->dev, "Failed to create direct bucket
at '%s' offset %lld\n",
storage->path, (long long) offset);
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index f528b3e19f21..7fa10f2eeb6b 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -650,14 +650,14 @@ struct state *state_new_from_node(struct device_node
*node, bool readonly)
if (ret)
goto out_release_state;
+ if (readonly)
+ state_backend_set_readonly(state);
+
ret = state_storage_init(state, state->backend_path, offset,
size, stridesize, storage_type);
if (ret)
goto out_release_state;
- if (readonly)
- state_backend_set_readonly(state);
-
ret = state_from_node(state, node, 1);
if (ret) {
goto out_release_state;
diff --git a/src/barebox-state/state.h b/src/barebox-state/state.h
index 912d6d484823..2e9b4b83f093 100644
--- a/src/barebox-state/state.h
+++ b/src/barebox-state/state.h
@@ -227,7 +227,8 @@ void state_backend_set_readonly(struct state *state);
void state_storage_free(struct state_backend_storage *storage);
int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
struct state_backend_storage_bucket
**bucket,
- off_t offset, ssize_t max_size);
+ off_t offset, ssize_t max_size,
+ bool readonly);
int state_storage_write(struct state_backend_storage *storage,
const void * buf, ssize_t len);
int state_storage_read(struct state_backend_storage *storage,
--
2.39.2
[OSS-Tools] [PATCH 3/3] libdt: use block device partition instead of parent if found
Linux doesn't parse MTD fixed-partitions described in the device tree
for block devices and EEPROMs. For block devices, The dt-utils work around
this by doing the lookup in userspace and then arrive at the parent block
device along with an offset that is passed along. We can do better
though: If there's a partition block device that contains the region
we are interested in, we should just be using that.
This avoids the issue of triggering a reread of the partition table
after writing barebox state because udev is notified that we had
been opening the whole block device writable.
Tested-by: Ulrich Ölmann
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 62 +
1 file changed, 48 insertions(+), 14 deletions(-)
diff --git a/src/libdt.c b/src/libdt.c
index 6f1a9e309d53..7d03ac1d4394 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2159,23 +2159,32 @@ static int udev_device_parse_sysattr_u64(struct
udev_device *dev, const char *at
return 0;
}
+/* True if A completely contains B */
+static bool region_contains(loff_t starta, loff_t enda,
+ loff_t startb, loff_t endb)
+{
+return starta <= startb && enda >= endb;
+}
+
/*
* device_find_block_device - extract device path from udev block device
*
* @dev: the udev_device to extract information from
* @devpath: returns the devicepath under which the block device is
accessible
+ * @start: start of partition in bytes
+ * @size: size of partition in bytes
*
- * returns 0 for success or negative error value on failure.
+ * returns 0 for success or negative error value on failure. Start
+ * is adjusted if a suitable partition is found within the parent block device.
*/
int device_find_block_device(struct udev_device *dev,
- char **devpath)
+ char **devpath, off_t *start, size_t size)
{
struct udev *udev;
struct udev_enumerate *enumerate;
struct udev_list_entry *devices, *dev_list_entry;
- struct udev_device *part;
- const char *outpath;
+ struct udev_device *part, *best_match = NULL;
int ret;
udev = udev_new();
@@ -2199,19 +2208,43 @@ int device_find_block_device(struct udev_device *dev,
if (!devtype)
continue;
if (!strcmp(devtype, "disk")) {
- outpath = udev_device_get_devnode(part);
- *devpath = strdup(outpath);
- ret = 0;
- goto out;
+ best_match = part;
+
+ /* Should we try to find a matching partition first? */
+ if (!size)
+ break;
+ } else if (start && size && !strcmp(devtype, "partition")) {
+ u64 partstart, partsize;
+
+ ret = udev_device_parse_sysattr_u64(part, "start",
);
+ if (ret)
+ continue;
+
+ ret = udev_device_parse_sysattr_u64(part, "size",
);
+ if (ret)
+ continue;
+
+ /* start/size sys attributes are always in 512-byte
units */
+ partstart *= 512;
+ partsize *= 512;
+
+ if (!region_contains(*start, *start + size,
+partstart, partstart + partsize))
+ continue;
+
+ best_match = part;
+ *start -= partstart;
+ break;
}
}
- ret = -ENODEV;
-out:
+ if (best_match)
+ *devpath = strdup(udev_device_get_devnode(best_match));
+
udev_enumerate_unref(enumerate);
udev_unref(udev);
- return ret;
+ return best_match ? 0 : -ENODEV;
}
/*
@@ -2428,7 +2461,7 @@ int of_get_devicepath(struct device_node *partition_node,
char **devpath, off_t
return udev_parse_eeprom(dev, devpath);
} else if (!udev_parse_mtd(dev, devpath, size)) {
return 0;
- } else if (device_find_block_device(dev, devpath)) {
+ } else if (device_find_block_device(dev, devpath, NULL, 0)) {
return of_parse_partition(partition_node, offset, size);
}
@@ -2505,10 +2538,11 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
return of_parse_partition(partition_node, offset, size);
} else {
- ret = device_find_block_device(dev, devpath);
+ ret = of_parse_partition(partition_node, offset, size);
if (ret)
return ret;
- return of_p
Re: [OSS-Tools] [PATCH v3 2/3] libdt: add support for barebox, storage-by-uuid
On 10.05.22 08:57, Michael Olbrich wrote:
> Signed-off-by: Michael Olbrich
> ---
>
> uuid comparison fixed:
> - compare the correct uuids
> - don't crash when no uuid is present
>
> src/libdt.c | 74 +
> 1 file changed, 69 insertions(+), 5 deletions(-)
>
> diff --git a/src/libdt.c b/src/libdt.c
> index 48c31931e8a1..f2c7c49ebcde 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2358,6 +2358,52 @@ out:
> return dev;
> }
>
> +static struct udev_device *of_find_device_by_uuid(const char *uuid)
> +{
> + struct udev *udev;
> + struct udev_enumerate *enumerate;
> + struct udev_list_entry *devices, *dev_list_entry;
> + int ret = 0;
> +
> + udev = udev_new();
> + if (!udev) {
> + fprintf(stderr, "Can't create udev\n");
> + return NULL;
> + }
> +
> + enumerate = udev_enumerate_new(udev);
> + udev_enumerate_add_match_subsystem(enumerate, "block");
> + udev_enumerate_scan_devices(enumerate);
> + devices = udev_enumerate_get_list_entry(enumerate);
> + udev_list_entry_foreach(dev_list_entry, devices) {
> + const char *path, *devtype, *outpath, *dev_uuid;
> + struct udev_device *device;
> +
> + path = udev_list_entry_get_name(dev_list_entry);
> + device = udev_device_new_from_syspath(udev, path);
> +
> + /* distinguish device (disk) from partitions */
> + devtype = udev_device_get_devtype(device);
> + if (!devtype)
> + continue;
> + if (!strcmp(devtype, "disk")) {
> + dev_uuid = udev_device_get_property_value(device,
> "ID_PART_TABLE_UUID");
> + if (dev_uuid && !strcmp(dev_uuid, uuid)) {
The partuuid comparison is case insensitive. Making UUID case-sensitive
might be a bit surprising for users.
> + outpath = udev_device_get_devnode(device);
> + return device;
> + }
> + } else if (!strcmp(devtype, "partition")) {
> + dev_uuid = udev_device_get_property_value(device,
> "ID_PART_ENTRY_UUID");
> + if (dev_uuid && !strcmp(dev_uuid, uuid)) {
> + outpath = udev_device_get_devnode(device);
> + return device;
> + }
> + }
> +
> + }
> + return NULL;
> +}
> +
> /*
> * of_get_devicepath - get information how to access device corresponding to
> a device_node
> * @partition_node: The device_node which shall be accessed
> @@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node
> *partition_node, char **devpath, off_t
> if (!strcmp(node->name, "partitions"))
> node = node->parent;
>
> - dev = of_find_device_by_node_path(node->full_name);
> - if (!dev) {
> - fprintf(stderr, "%s: cannot find device from node %s\n",
> __func__,
> - node->full_name);
> - return -ENODEV;
> + if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
> + const char *uuid;
> +
> + ret = of_property_read_string(node, "uuid", );
> + if (ret) {
> + fprintf(stderr, "%s: missing uuid property for %s\n",
> __func__,
> + node->full_name);
> + return -ENODEV;
> + }
> + dev = of_find_device_by_uuid(uuid);
> + if (!dev) {
> + fprintf(stderr, "%s: cannot find device for uuid %s\n",
> __func__,
> + uuid);
> + return -ENODEV;
> + }
> + }
> + else {
> + dev = of_find_device_by_node_path(node->full_name);
> + if (!dev) {
> + fprintf(stderr, "%s: cannot find device from node
> %s\n", __func__,
> + node->full_name);
> + return -ENODEV;
> + }
> }
>
> mtd = of_find_mtd_device(dev);
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH v3 3/3] state: automatically find state.dtb in the ESP
Hello Michael,
On 10.05.22 08:57, Michael Olbrich wrote:
> Systemd mounts the EFI System Partition (ESP) to /boot or /efi.
> So look there for the state.dtb when the devicetree in sysfs/procfs is
> not available.
>
> This way barebox-state can be used on EFI systems without manually
> specifying the devicetree file.
>
> Signed-off-by: Michael Olbrich
> ---
> src/barebox-state.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/src/barebox-state.c b/src/barebox-state.c
> index 334aed6f3d43..bf67340d4dc6 100644
> --- a/src/barebox-state.c
> +++ b/src/barebox-state.c
> @@ -342,6 +342,30 @@ struct state *state_get(const char *name, const char
> *filename, bool readonly, b
> }
> } else {
> root = of_read_proc_devicetree();
> +
> + /* No device-tree in procfs / sysfs, try dtb file in the ESP */
> + if (-PTR_ERR(root) == ENOENT) {
> + const char *paths[] = {
> + /* default mount paths used by systemd */
> + "/boot/EFI/BAREBOX/state.dtb",
> + "/efi/EFI/BAREBOX/state.dtb",
On my Debian, it is /boot/efi/EFI. Do you mind appending that one to the list?
:-)
> + NULL
> + };
> + void *fdt;
> + int i;
> +
> + for (i = 0; paths[i]; ++i) {
> + fdt = read_file(paths[i], NULL);
> + if (fdt)
> + break;
> + }
> + if (fdt) {
> + root = of_unflatten_dtb(fdt);
> + free(fdt);
> + }
> + else
> + root = ERR_PTR(-ENOENT);
This duplicates code in the first if clause. Can you move this into
a common helper (just a static function above state_get suffices)
and use it?
Thanks,
Ahmad
> + }
> if (IS_ERR(root)) {
> pr_err("Unable to read devicetree. %s\n",
> strerror(-PTR_ERR(root)));
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [OSS-Tools] [PATCH dt-utils 1/4] treewide: add SPDX identifiers to files with GPL-2.0-or-later license
Hello Roland, On 30.03.21 17:53, Roland Hieber wrote: > On Tue, Mar 30, 2021 at 03:50:19PM +0200, Ahmad Fatoum wrote: >> Hello, >> >> On 30.03.21 15:30, Uwe Kleine-König wrote: >>> Hello, >>> >>> On Tue, Mar 30, 2021 at 01:22:26PM +0200, Ahmad Fatoum wrote: >>>> On 30.03.21 13:08, Roland Hieber wrote: >>>>> Uwe, could I get a Reviewed-by from you for these four patches? :-) >>>> >>>> We must still carry the full license texts in the project (cf. Linux >>>> LICENSES/). This is missing here, no? >>> >>> well, it depends on what you want to achieve. If you want to become SPDX >>> conformant this is indeed necessary. IMHO the conversion from several >>> different boilerplate license specifications to a single line is >>> beneficial even if the full licenses are not (yet) in the project. >> >> If the code says that the license terms need to be distributed along >> with the software, you are violating the license terms, if you don't >> abide by this. This is unrelated to SPDX-Conformance. > > Nothing in the license headers or in the GPL or in the Zlib license says > that we have to distribute the license text along with the source code. > On the contrary, there are even some versions of GPL license headers > that have a clause "if you didn't a license text along with the > software, write to the FSF at $ADDRESS". If Zlib says "This notice may not be removed or altered from any source distribution.", I interpret this as meaning that we need to ship the full text as part of the source distribution and only a SPDX-License-Identifier that can be looked up on a website somewhere doesn't suffice. > But it is needed for REUSE [1] compliance, so I intend to move COPYING > into LICENSES/ nevertheless, and add the Zlib too. Thanks for catching > this. :) Cheers, Ahmad > > [1]: https://reuse.software/ > > - Roland > >>> So I don't regret having given my Review-tags. >> >> Reviewing is fine, but before applying license text removal, the full >> license texts need to be located in full somewhere in the source tree. >> >> Cheers, >> Ahmad >> >>> >>> Best regards >>> Uwe >>> >> >> -- >> Pengutronix e.K. | | >> Steuerwalder Str. 21 | http://www.pengutronix.de/ | >> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| >> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
Re: [OSS-Tools] [PATCH dt-utils 1/4] treewide: add SPDX identifiers to files with GPL-2.0-or-later license
On 30.03.21 16:00, Uwe Kleine-König wrote: > Hi Ahmad, > > On Tue, Mar 30, 2021 at 03:50:19PM +0200, Ahmad Fatoum wrote: >> On 30.03.21 15:30, Uwe Kleine-König wrote: >>> On Tue, Mar 30, 2021 at 01:22:26PM +0200, Ahmad Fatoum wrote: >>>> On 30.03.21 13:08, Roland Hieber wrote: >>>>> Uwe, could I get a Reviewed-by from you for these four patches? :-) >>>> >>>> We must still carry the full license texts in the project (cf. Linux >>>> LICENSES/). This is missing here, no? >>> >>> well, it depends on what you want to achieve. If you want to become SPDX >>> conformant this is indeed necessary. IMHO the conversion from several >>> different boilerplate license specifications to a single line is >>> beneficial even if the full licenses are not (yet) in the project. >> >> If the code says that the license terms need to be distributed along >> with the software, you are violating the license terms, if you don't >> abide by this. This is unrelated to SPDX-Conformance. > > If distributing the full license text is required, this was a problem > already before, wasn't it? Hmm, indeed. GPL-2.0 license text exists in COPYING. Zlib license text doesn't exist in full. > > Best regards > Uwe -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
Re: [OSS-Tools] [PATCH dt-utils 1/4] treewide: add SPDX identifiers to files with GPL-2.0-or-later license
Hello, On 30.03.21 15:30, Uwe Kleine-König wrote: > Hello, > > On Tue, Mar 30, 2021 at 01:22:26PM +0200, Ahmad Fatoum wrote: >> On 30.03.21 13:08, Roland Hieber wrote: >>> Uwe, could I get a Reviewed-by from you for these four patches? :-) >> >> We must still carry the full license texts in the project (cf. Linux >> LICENSES/). This is missing here, no? > > well, it depends on what you want to achieve. If you want to become SPDX > conformant this is indeed necessary. IMHO the conversion from several > different boilerplate license specifications to a single line is > beneficial even if the full licenses are not (yet) in the project. If the code says that the license terms need to be distributed along with the software, you are violating the license terms, if you don't abide by this. This is unrelated to SPDX-Conformance. > So I don't regret having given my Review-tags. Reviewing is fine, but before applying license text removal, the full license texts need to be located in full somewhere in the source tree. Cheers, Ahmad > > Best regards > Uwe > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
Re: [OSS-Tools] [PATCH dt-utils 1/4] treewide: add SPDX identifiers to files with GPL-2.0-or-later license
Hello Roland, On 30.03.21 13:08, Roland Hieber wrote: > Uwe, could I get a Reviewed-by from you for these four patches? :-) We must still carry the full license texts in the project (cf. Linux LICENSES/). This is missing here, no? Cheers, Ahmad > > - Roland > > On Fri, Mar 26, 2021 at 10:06:44PM +0100, Roland Hieber wrote: >> Signed-off-by: Roland Hieber >> --- >> sizes.h | 12 +--- >> src/barebox-state/state.c | 11 +-- >> src/barebox-state/state_variables.c | 11 +-- >> src/base64.c| 3 +-- >> src/crypto/sha1.c | 7 +-- >> src/crypto/sha2.c | 7 +-- >> 6 files changed, 6 insertions(+), 45 deletions(-) >> >> diff --git a/sizes.h b/sizes.h >> index 6f91e9b4bd23..a73bb87e4b5b 100644 >> --- a/sizes.h >> +++ b/sizes.h >> @@ -1,14 +1,4 @@ >> -/* >> - * This program is free software; you can redistribute it and/or modify >> - * it under the terms of the GNU General Public License as published by >> - * the Free Software Foundation; either version 2 of the License, or >> - * (at your option) any later version. >> - * >> - * This program is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> - * GNU General Public License for more details. >> - */ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> /* Size defintions >> * Copyright (C) ARM Limited 1998. All rights reserved. >> */ >> diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c >> index f528b3e19f21..e3825d6aeab2 100644 >> --- a/src/barebox-state/state.c >> +++ b/src/barebox-state/state.c >> @@ -1,17 +1,8 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> /* >> * Copyright (C) 2012-2014 Pengutronix, Jan Luebbe >> * Copyright (C) 2013-2014 Pengutronix, Sascha Hauer >> >> * Copyright (C) 2015 Pengutronix, Marc Kleine-Budde >> - * >> - * This program is free software; you can redistribute it and/or modify >> - * it under the terms of the GNU General Public License as published by >> - * the Free Software Foundation; either version 2 of the License, or >> - * (at your option) any later version. >> - * >> - * This program is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> - * GNU General Public License for more details. >> */ >> >> #include >> diff --git a/src/barebox-state/state_variables.c >> b/src/barebox-state/state_variables.c >> index 16f630f57f9f..429f1f0a3479 100644 >> --- a/src/barebox-state/state_variables.c >> +++ b/src/barebox-state/state_variables.c >> @@ -1,17 +1,8 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> /* >> * Copyright (C) 2012-2014 Pengutronix, Jan Luebbe >> * Copyright (C) 2013-2014 Pengutronix, Sascha Hauer >> >> * Copyright (C) 2015 Pengutronix, Marc Kleine-Budde >> - * >> - * This program is free software; you can redistribute it and/or modify >> - * it under the terms of the GNU General Public License as published by >> - * the Free Software Foundation; either version 2 of the License, or >> - * (at your option) any later version. >> - * >> - * This program is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> - * GNU General Public License for more details. >> */ >> >> #include >> diff --git a/src/base64.c b/src/base64.c >> index 6c02174d6377..5c7f9e884965 100644 >> --- a/src/base64.c >> +++ b/src/base64.c >> @@ -1,11 +1,10 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> /* >> * Code based on busybox-1.23.2 >> * >> * Copyright 2003, Glenn McGrath >> * Copyright 2006, Rob Landley >> * Copyright 2010, Denys Vlasenko >> - * >> - * Licensed under GPLv2 or later, see file LICENSE in this tarball for >> details. >> */ >> >> #include >> diff --git a/src/crypto/sha1.c b/src/crypto/sha1.c >> index cbde4d28e475..a66f4f2505d2 100644 >> --- a/src/crypto/sha1.c >> +++ b/src/crypto/sha1.c >> @@ -1,3 +1,4 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> /* >> * Cryptographic API. >> * >> @@ -9,12 +10,6 @@ >> * Copyright (c) Alan Smithee. >> * Copyright (c) Andrew McDonald >> * Copyright (c) Jean-Francois Dive >> - * >> - * This program is free software; you can redistribute it and/or modify it >> - * under the terms of the GNU General Public License as published by the >> Free >> - * Software Foundation; either version 2 of the License, or (at your option) >> - * any later version. >> - * >> */ >> >> #include >> diff --git a/src/crypto/sha2.c b/src/crypto/sha2.c >> index cb0f11c77ea0..4b3da0c9d230 100644 >> --- a/src/crypto/sha2.c >> +++ b/src/crypto/sha2.c
Re: [OSS-Tools] [PATCH] barebox-state: introduce --no-lock option to avoid lock file
Hello,
On 09.07.20 15:31, Ahmad Fatoum wrote:
> barebox-state may be run in restricted contexts where /var/lock is not
> yet mounted. Introduce a --no-lock option for users that guarantee by
> other means that multiple instances of barebox-state won't run concurrently.
Please dismiss. This patch was for running barebox-state in a systemd-generator,
which didn't have /var/lock available. /run is however available that early,
so I replaced this patch with Agner's lock in /run patch on my side and
I no longer see a need for this to be merged.
>
> Signed-off-by: Ahmad Fatoum
> ---
> Correct operation can be verified with
> $ strace barebox-state -i file.dtb --no-lock 2>&1 | grep /var/lock
> ---
> src/barebox-state.c | 35 +++
> 1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/src/barebox-state.c b/src/barebox-state.c
> index c7f6dee90d4a..cd56ce7192c3 100644
> --- a/src/barebox-state.c
> +++ b/src/barebox-state.c
> @@ -381,6 +381,7 @@ struct state *state_get(const char *name, const char
> *filename, bool readonly, b
> enum opt {
> OPT_DUMP_SHELL = UCHAR_MAX + 1,
> OPT_VERSION= UCHAR_MAX + 2,
> + OPT_NO_LOCK= UCHAR_MAX + 3,
> };
>
> static struct option long_options[] = {
> @@ -391,6 +392,7 @@ static struct option long_options[] = {
> {"dump",no_argument,0, 'd' },
> {"dump-shell", no_argument,0, OPT_DUMP_SHELL },
> {"force", no_argument,0, 'f' },
> + {"no-lock", no_argument,0, OPT_NO_LOCK },
> {"verbose", no_argument,0, 'v' },
> {"quiet", no_argument,0, 'q' },
> {"version", no_argument,0, OPT_VERSION },
> @@ -410,6 +412,7 @@ static void usage(char *name)
> "-d, --dumpdump the state\n"
> "--dump-shell dump the state suitable for shell
> sourcing\n"
> "-f, --force do not check for state
> manipulation via the HMAC\n"
> +"--no-lock do not use lock file to guard
> access\n"
> "-v, --verbose increase verbosity\n"
> "-q, --quiet decrease verbosity\n"
> "--version display version\n"
> @@ -447,6 +450,7 @@ int main(int argc, char *argv[])
> int pr_level = 5;
> int auth = 1;
> const char *dtb = NULL;
> + bool use_lock_file = true;
>
> INIT_LIST_HEAD(_list);
> INIT_LIST_HEAD(_list.list);
> @@ -478,6 +482,9 @@ int main(int argc, char *argv[])
> case 'f':
> auth = 0;
> break;
> + case OPT_NO_LOCK:
> + use_lock_file = false;
> + break;
> case 'd':
> do_dump = 1;
> break;
> @@ -530,17 +537,19 @@ int main(int argc, char *argv[])
> ++nr_states;
> }
>
> - lock_fd = open("/var/lock/barebox-state", O_CREAT | O_RDWR, 0600);
> - if (lock_fd < 0) {
> - pr_err("Failed to open lock-file /var/lock/barebox-state\n");
> - exit(1);
> - }
> + if (use_lock_file) {
> + lock_fd = open("/var/lock/barebox-state", O_CREAT | O_RDWR,
> 0600);
> + if (lock_fd < 0) {
> + pr_err("Failed to open lock-file
> /var/lock/barebox-state\n");
> + exit(1);
> + }
>
> - ret = flock(lock_fd, LOCK_EX);
> - if (ret < 0) {
> - pr_err("Failed to lock /var/lock/barebox-state: %m\n");
> - close(lock_fd);
> - exit(1);
> + ret = flock(lock_fd, LOCK_EX);
> + if (ret < 0) {
> + pr_err("Failed to lock /var/lock/barebox-state: %m\n");
> + close(lock_fd);
> + exit(1);
> + }
> }
>
> list_for_each_entry(state, _list.list, list) {
> @@ -662,8 +671,10 @@ int main(int argc, char *argv[])
>
> ret = 0;
> out_unlock:
> - flock(lock_fd, LOCK_UN);
> - close(lock_fd);
> + if (use_lock_file) {
> + flock(lock_fd, LOCK_UN);
> + close(lock_fd);
> + }
>
> return ret;
> }
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
OSS-Tools mailing list
OSS-Tools@pengutronix.de
Re: [OSS-Tools] [PATCH dt-utils] state: use /run to store lockfile
On 29.11.20 22:11, Stefan Agner wrote:
> The current location /var/lock is considered legacy (at least by
> systemd). Just use /run to store the lockfile and append the usual .lock
> suffix.
>
> Signed-off-by: Stefan Agner
Tested-by: Ahmad Fatoum
> ---
> src/barebox-state.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/barebox-state.c b/src/barebox-state.c
> index 946a8db..16a8c9f 100644
> --- a/src/barebox-state.c
> +++ b/src/barebox-state.c
> @@ -38,6 +38,8 @@
> #include
> #include
>
> +#define BAREBOX_STATE_LOCKFILE "/run/barebox-state.lock"
> +
> struct state_variable;
>
> static int __state_uint8_set(struct state_variable *var, const char
> *val);
> @@ -505,15 +507,15 @@ int main(int argc, char *argv[])
> ++nr_states;
> }
>
> - lock_fd = open("/var/lock/barebox-state", O_CREAT | O_RDWR, 0600);
> + lock_fd = open(BAREBOX_STATE_LOCKFILE, O_CREAT | O_RDWR, 0600);
> if (lock_fd < 0) {
> - pr_err("Failed to open lock-file /var/lock/barebox-state\n");
> + pr_err("Failed to open lock-file " BAREBOX_STATE_LOCKFILE "\n");
> exit(1);
> }
>
> ret = flock(lock_fd, LOCK_EX);
> if (ret < 0) {
> - pr_err("Failed to lock /var/lock/barebox-state: %m\n");
> + pr_err("Failed to lock " BAREBOX_STATE_LOCKFILE ": %m\n");
> close(lock_fd);
> exit(1);
> }
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
OSS-Tools mailing list
OSS-Tools@pengutronix.de
[OSS-Tools] [PATCH v2] barebox-state: have the --set option to avoid writes if possible
barebox-state --set always dirties the state when successful. Users
seeking to conserve write cycles thus have to --get the variable in
question first to check whether to write it. Make life of such users
easier by having barebox-state support this out-of-the-box.
This allows users to fire and forget execute barebox-state.
This arguably should have been the behavior from the beginning,
the state implementation shared by barebox and dt-utils already
marks the state dirty if buckets appear corrupted on probe, so
there is no extra benefit in always executing the write.
The comparison to determine whether the state should be dirtied
does an extra allocation in interest of clarity.
This overhead is deemed negligible compared to I/O and it makes
the code easier to follow.
Suggested-by: Jan Lübbe
Signed-off-by: Ahmad Fatoum
---
v1 -> v2:
- incorporate Jan's (off-list) suggestion to just change --set
behavior. state implementation already dirties state if a
bucket is corrupt, so there is really no valid use case for
not conserving writes by default.
---
src/barebox-state.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/barebox-state.c b/src/barebox-state.c
index cd56ce7192c3..7b5c0dae00dd 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -283,6 +283,7 @@ static int state_set_var(struct state *state, const char
*var, const char *val)
{
struct state_variable *sv;
struct variable_str_type *vtype;
+ char *oldval;
int ret;
sv = state_find_var(state, var);
@@ -296,6 +297,14 @@ static int state_set_var(struct state *state, const char
*var, const char *val)
if (!vtype->set)
return -EPERM;
+ oldval = vtype->get(sv);
+ if (!IS_ERR(oldval)) {
+ bool equal = strcmp(oldval, val) == 0;
+ free(oldval);
+ if (equal)
+ return 0;
+ }
+
ret = vtype->set(sv, val);
if (ret)
return ret;
--
2.28.0
___
OSS-Tools mailing list
OSS-Tools@pengutronix.de
[OSS-Tools] [PATCH] barebox-state: add new --update option to avoid writes if possible
barebox-state --set always writes the state when successful. Users
seeking to conserve write cycles thus have to --get the variable in
question first to check whether to write it. Make life of such users
easier by having barebox-state support this out-of-the-box.
Use of the new --update option will only write a variable if its
new and old value compare unequal. This allows users to fire and
forget execute barebox-state.
Signed-off-by: Ahmad Fatoum
---
src/barebox-state.c | 32 +---
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/src/barebox-state.c b/src/barebox-state.c
index cd56ce7192c3..09e054e25f8b 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -51,6 +51,8 @@ static char *__state_mac_get(struct state_variable *var);
static int __state_string_set(struct state_variable *var, const char *val);
static char *__state_string_get(struct state_variable *var);
+enum state_action { STATE_ACTION_GET, STATE_ACTION_SET, STATE_ACTION_UPDATE };
+
struct variable_str_type {
enum state_variable_type type;
char *type_name;
@@ -279,7 +281,8 @@ char *state_get_var(struct state *state, const char *var)
return vtype->get(sv);
}
-static int state_set_var(struct state *state, const char *var, const char *val)
+static int state_set_var(struct state *state, const char *var, const char *val,
+enum state_action action)
{
struct state_variable *sv;
struct variable_str_type *vtype;
@@ -296,6 +299,18 @@ static int state_set_var(struct state *state, const char
*var, const char *val)
if (!vtype->set)
return -EPERM;
+ if (action == STATE_ACTION_UPDATE) {
+ char *oldval = vtype->get(sv);
+
+ if (!IS_ERR(oldval)) {
+ if (!strcmp(oldval, val)) {
+ free(oldval);
+ return 0;
+ }
+ free(oldval);
+ }
+ }
+
ret = vtype->set(sv, val);
if (ret)
return ret;
@@ -387,6 +402,7 @@ enum opt {
static struct option long_options[] = {
{"get", required_argument, 0, 'g' },
{"set", required_argument, 0, 's' },
+ {"update", required_argument, 0, 'u' },
{"name",required_argument, 0, 'n' },
{"input", required_argument, 0, 'i' },
{"dump",no_argument,0, 'd' },
@@ -407,6 +423,7 @@ static void usage(char *name)
"\n"
"-g, --get get the value of a variable\n"
"-s, --set = set the value of a variable\n"
+"-u, --update = update the value of a variable only
if it changed\n"
"-n, --name specify the state to use
(default=\"state\"). Multiple states are allowed.\n"
"-i, --input load the devicetree from a file
instead of using the system devicetree.\n"
"-d, --dumpdump the state\n"
@@ -425,7 +442,7 @@ static void usage(char *name)
struct state_set_get {
char *arg;
- int get;
+ enum state_action action;
struct list_head list;
};
@@ -456,7 +473,7 @@ int main(int argc, char *argv[])
INIT_LIST_HEAD(_list.list);
while (1) {
- c = getopt_long(argc, argv, "hg:s:i:dvn:qf", long_options,
_index);
+ c = getopt_long(argc, argv, "hg:u:s:i:dvn:qf", long_options,
_index);
if (c < 0)
break;
switch (c) {
@@ -468,13 +485,14 @@ int main(int argc, char *argv[])
exit(0);
case 'g':
sg = xzalloc(sizeof(*sg));
- sg->get = 1;
+ sg->action = STATE_ACTION_GET;
sg->arg = optarg;
list_add_tail(>list, _list);
break;
+ case 'u':
case 's':
sg = xzalloc(sizeof(*sg));
- sg->get = 0;
+ sg->action = c == 's' ? STATE_ACTION_SET :
STATE_ACTION_UPDATE;
sg->arg = optarg;
list_add_tail(>list, _list);
readonly = false;
@@ -627,7 +645,7 @@ int main(int argc, char *argv[])
if (state == _list) {
state = list_first_entry(_list.list, struct
state_list, list);
}
- if (sg->get) {
+ if (sg->action == STATE_ACTION_GET) {
char *val = state_get_var(state->state, arg);
[OSS-Tools] [PATCH] libdt: enumerate amba bus as well
The STM32MP1 probes the SD/MMC host controller over amba, not the platform bus as most other ARM systems. Enumerate amba as well, so we can use barebox-state on that SoC. Reported-by: Jookia Tested-by: Xogium Signed-off-by: Ahmad Fatoum --- src/libdt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libdt.c b/src/libdt.c index 01f0a6941aa2..342313f8aed7 100644 --- a/src/libdt.c +++ b/src/libdt.c @@ -2057,6 +2057,7 @@ static void of_scan_udev_devices(void) udev_enumerate_add_match_subsystem(enumerate, "i2c"); udev_enumerate_add_match_subsystem(enumerate, "spi"); udev_enumerate_add_match_subsystem(enumerate, "mtd"); + udev_enumerate_add_match_subsystem(enumerate, "amba"); udev_enumerate_scan_devices(enumerate); devices = udev_enumerate_get_list_entry(enumerate); -- 2.28.0 ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
[OSS-Tools] [PATCH] barebox-state: introduce --no-lock option to avoid lock file
barebox-state may be run in restricted contexts where /var/lock is not
yet mounted. Introduce a --no-lock option for users that guarantee by
other means that multiple instances of barebox-state won't run concurrently.
Signed-off-by: Ahmad Fatoum
---
Correct operation can be verified with
$ strace barebox-state -i file.dtb --no-lock 2>&1 | grep /var/lock
---
src/barebox-state.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/src/barebox-state.c b/src/barebox-state.c
index c7f6dee90d4a..cd56ce7192c3 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -381,6 +381,7 @@ struct state *state_get(const char *name, const char
*filename, bool readonly, b
enum opt {
OPT_DUMP_SHELL = UCHAR_MAX + 1,
OPT_VERSION= UCHAR_MAX + 2,
+ OPT_NO_LOCK= UCHAR_MAX + 3,
};
static struct option long_options[] = {
@@ -391,6 +392,7 @@ static struct option long_options[] = {
{"dump",no_argument,0, 'd' },
{"dump-shell", no_argument,0, OPT_DUMP_SHELL },
{"force", no_argument,0, 'f' },
+ {"no-lock", no_argument,0, OPT_NO_LOCK },
{"verbose", no_argument,0, 'v' },
{"quiet", no_argument,0, 'q' },
{"version", no_argument,0, OPT_VERSION },
@@ -410,6 +412,7 @@ static void usage(char *name)
"-d, --dumpdump the state\n"
"--dump-shell dump the state suitable for shell
sourcing\n"
"-f, --force do not check for state manipulation
via the HMAC\n"
+"--no-lock do not use lock file to guard
access\n"
"-v, --verbose increase verbosity\n"
"-q, --quiet decrease verbosity\n"
"--version display version\n"
@@ -447,6 +450,7 @@ int main(int argc, char *argv[])
int pr_level = 5;
int auth = 1;
const char *dtb = NULL;
+ bool use_lock_file = true;
INIT_LIST_HEAD(_list);
INIT_LIST_HEAD(_list.list);
@@ -478,6 +482,9 @@ int main(int argc, char *argv[])
case 'f':
auth = 0;
break;
+ case OPT_NO_LOCK:
+ use_lock_file = false;
+ break;
case 'd':
do_dump = 1;
break;
@@ -530,17 +537,19 @@ int main(int argc, char *argv[])
++nr_states;
}
- lock_fd = open("/var/lock/barebox-state", O_CREAT | O_RDWR, 0600);
- if (lock_fd < 0) {
- pr_err("Failed to open lock-file /var/lock/barebox-state\n");
- exit(1);
- }
+ if (use_lock_file) {
+ lock_fd = open("/var/lock/barebox-state", O_CREAT | O_RDWR,
0600);
+ if (lock_fd < 0) {
+ pr_err("Failed to open lock-file
/var/lock/barebox-state\n");
+ exit(1);
+ }
- ret = flock(lock_fd, LOCK_EX);
- if (ret < 0) {
- pr_err("Failed to lock /var/lock/barebox-state: %m\n");
- close(lock_fd);
- exit(1);
+ ret = flock(lock_fd, LOCK_EX);
+ if (ret < 0) {
+ pr_err("Failed to lock /var/lock/barebox-state: %m\n");
+ close(lock_fd);
+ exit(1);
+ }
}
list_for_each_entry(state, _list.list, list) {
@@ -662,8 +671,10 @@ int main(int argc, char *argv[])
ret = 0;
out_unlock:
- flock(lock_fd, LOCK_UN);
- close(lock_fd);
+ if (use_lock_file) {
+ flock(lock_fd, LOCK_UN);
+ close(lock_fd);
+ }
return ret;
}
--
2.27.0
___
OSS-Tools mailing list
OSS-Tools@pengutronix.de
Re: [OSS-Tools] [PATCH dt-utils v5 0/2] barebox-state: get devicetree from file
On 10/24/19 4:24 PM, Ahmad Fatoum wrote: > For use on systems that don't normally have a device tree, like x86, > it would be nice to be able to pass barebox_state the path to a device > tree blob directly, which is what this patch set does. Gentle ping. > > Cheers > Ahmad > > Changes in v5: > - Rebased onto current master > - Added commit to handle non-lower-case partuuids > - Fixed use after free by not allocating dynamic memory > > Ahmad Fatoum (1): > libdt: support upper-case hexadecimals in value of partuuid property > > Steffen Trumtrar (1): > barebox-state: get devicetree from file > > src/barebox-state.c | 41 + > src/barebox-state.h | 2 +- > src/keystore-blob.c | 2 +- > src/libdt.c | 12 +++- > 4 files changed, 46 insertions(+), 11 deletions(-) > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
[OSS-Tools] [PATCH v5 2/2] barebox-state: get devicetree from file
From: Steffen Trumtrar
Adds a -i/--input argument to barebox-state to allow passing a
devicetree as a file instead of using it from the system.
This can be used for example on systems that do not use device trees
(such as x86) but where we want to use a dtb blob for describing the
state storage and format.
Signed-off-by: Steffen Trumtrar
Signed-off-by: Enrico Jorns
Signed-off-by: Ahmad Fatoum
---
src/barebox-state.c | 41 +
src/barebox-state.h | 2 +-
src/keystore-blob.c | 2 +-
3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/src/barebox-state.c b/src/barebox-state.c
index 946a8dba6d8c..c7f6dee90d4a 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -306,17 +306,36 @@ static int state_set_var(struct state *state, const char
*var, const char *val)
}
-struct state *state_get(const char *name, bool readonly, bool auth)
+struct state *state_get(const char *name, const char *filename, bool readonly,
bool auth)
{
struct device_node *root, *node;
struct state *state;
int ret;
- root = of_read_proc_devicetree();
- if (IS_ERR(root)) {
- pr_err("Unable to read devicetree. %s\n",
- strerror(-PTR_ERR(root)));
- return ERR_CAST(root);
+ if (filename) {
+ void *fdt;
+
+ fdt = read_file(filename, NULL);
+ if (!fdt) {
+ pr_err("Unable to read devicetree file '%s'\n",
+ filename);
+ return ERR_PTR(-ENOENT);
+ }
+
+ root = of_unflatten_dtb(fdt);
+ free(fdt);
+ if (IS_ERR(root)) {
+ pr_err("Unable to read devicetree. %s\n",
+ strerror(-PTR_ERR(root)));
+ return ERR_CAST(root);
+ }
+ } else {
+ root = of_read_proc_devicetree();
+ if (IS_ERR(root)) {
+ pr_err("Unable to read devicetree. %s\n",
+ strerror(-PTR_ERR(root)));
+ return ERR_CAST(root);
+ }
}
of_set_root_node(root);
@@ -368,6 +387,7 @@ static struct option long_options[] = {
{"get", required_argument, 0, 'g' },
{"set", required_argument, 0, 's' },
{"name",required_argument, 0, 'n' },
+ {"input", required_argument, 0, 'i' },
{"dump",no_argument,0, 'd' },
{"dump-shell", no_argument,0, OPT_DUMP_SHELL },
{"force", no_argument,0, 'f' },
@@ -386,6 +406,7 @@ static void usage(char *name)
"-g, --get get the value of a variable\n"
"-s, --set = set the value of a variable\n"
"-n, --name specify the state to use
(default=\"state\"). Multiple states are allowed.\n"
+"-i, --input load the devicetree from a file
instead of using the system devicetree.\n"
"-d, --dumpdump the state\n"
"--dump-shell dump the state suitable for shell
sourcing\n"
"-f, --force do not check for state manipulation
via the HMAC\n"
@@ -425,12 +446,13 @@ int main(int argc, char *argv[])
bool readonly = true;
int pr_level = 5;
int auth = 1;
+ const char *dtb = NULL;
INIT_LIST_HEAD(_list);
INIT_LIST_HEAD(_list.list);
while (1) {
- c = getopt_long(argc, argv, "hg:s:dvn:qf", long_options,
_index);
+ c = getopt_long(argc, argv, "hg:s:i:dvn:qf", long_options,
_index);
if (c < 0)
break;
switch (c) {
@@ -479,6 +501,9 @@ int main(int argc, char *argv[])
++nr_states;
break;
}
+ case 'i':
+ dtb = optarg;
+ break;
case ':':
case '?':
default:
@@ -519,7 +544,7 @@ int main(int argc, char *argv[])
}
list_for_each_entry(state, _list.list, list) {
- state->state = state_get(state->name, readonly, auth);
+ state->state = state_get(state->name, dtb, readonly, auth);
if (!IS_ERR(state->state) && !state->name)
state->name = state->state->name;
if (IS_ERR(state->state)) {
diff --git a/src/barebox-state.h b/src/barebox-state.h
index b
[OSS-Tools] [PATCH v5 1/2] libdt: support upper-case hexadecimals in value of partuuid property
barebox/next now has a commit[1] to support the hexadecimal numbers in the
partition UUID to be upper case as well. This aligns its behavior with the
Linux kernel, where the PARTUUID kernel argument flag also supports[2]
lower and upper case UUIDs.
The UUIDs used in the /dev/disk/by-partuuid/* symlinks created by udev
come from libblkid, which formats the UUID hexadecimals as lower case[3],
so we only need to lower case the property value inside dt-utils to
have it support upper case part UUIDs as well.
[1]: "fs: devfs-core: do a case-insensitive compare of partuuids",
https://www.spinics.net/lists/u-boot-v2/msg39590.html
[2]: v5.4-rc4, init/do_mounts.c: match_dev_by_uuid()
[3]: util-linux v2.34, $(egrep -r '%[^\s"]+X' libblkid/)
Signed-off-by: Ahmad Fatoum
---
src/libdt.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/libdt.c b/src/libdt.c
index bdfd409b1aab..01f0a6941aa2 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2423,7 +2423,17 @@ int of_get_devicepath(struct device_node
*partition_node, char **devpath, off_t
/* when partuuid is specified short-circuit the search for the
cdev */
ret = of_property_read_string(partition_node, "partuuid",
);
if (!ret) {
- *devpath = basprintf("/dev/disk/by-partuuid/%s", uuid);
+ const char prefix[] = "/dev/disk/by-partuuid/";
+ size_t prefix_len = sizeof(prefix) - 1;
+ char *lc_uuid, *s;
+
+ lc_uuid = xzalloc(prefix_len + strlen(uuid) + 1);
+ s = memcpy(lc_uuid, prefix, prefix_len) + prefix_len;
+
+ while (*uuid)
+ *s++ = tolower(*uuid++);
+
+ *devpath = lc_uuid;
return 0;
}
--
2.23.0
___
OSS-Tools mailing list
OSS-Tools@pengutronix.de
[OSS-Tools] [PATCH dt-utils v5 0/2] barebox-state: get devicetree from file
For use on systems that don't normally have a device tree, like x86, it would be nice to be able to pass barebox_state the path to a device tree blob directly, which is what this patch set does. Cheers Ahmad Changes in v5: - Rebased onto current master - Added commit to handle non-lower-case partuuids - Fixed use after free by not allocating dynamic memory Ahmad Fatoum (1): libdt: support upper-case hexadecimals in value of partuuid property Steffen Trumtrar (1): barebox-state: get devicetree from file src/barebox-state.c | 41 + src/barebox-state.h | 2 +- src/keystore-blob.c | 2 +- src/libdt.c | 12 +++- 4 files changed, 46 insertions(+), 11 deletions(-) -- 2.23.0 ___ OSS-Tools mailing list OSS-Tools@pengutronix.de
