| Message ID | 69a1b78886392bca426ac6f521197af06d768042.1567326213.git.noring@nocrew.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Linux for the PlayStation 2 | expand |
Hello! On 01.09.2019 18:50, Fredrik Noring wrote: > Reading ROM files is trivial since they are permanently available in > memory. Having rom_read_file() is a convenient when for example > resolving the machine region in subsequent changes. > > Signed-off-by: Fredrik Noring <noring@nocrew.org> [...] > index 12a57f24bd63..840d37a199d8 100644 > --- a/arch/mips/ps2/rom.c > +++ b/arch/mips/ps2/rom.c > @@ -224,6 +224,39 @@ struct rom_file rom_first_file(const struct rom_dir dir) > } > EXPORT_SYMBOL_GPL(rom_first_file); > > +/** > + * rom_read_file - read ROM file data > + * @dir: directory to read the file from > + * @name: file name to read > + * @buffer: pointer to buffer to store data that is read > + * @size: size in bytes to read > + * @offset: offset in bytes to start reading > + * > + * Context: any > + * Return: on successful completion, a nonnegative integer indicating the > + * number of bytes actually read; otherwise, a negative error number > + */ > +ssize_t rom_read_file(const struct rom_dir dir, > + const char *name, void *buffer, size_t size, loff_t offset) > +{ > + struct rom_file file; > + > + rom_find_files(file, dir, name) > + if (offset < file.size) { > + const u8 *b = file.data; > + size_t remaining = file.size - offset; > + size_t n = min(size, remaining); > + > + memcpy(buffer, &b[offset], n); > + > + return n; > + } else The *else* branch also needs {} if the *if* branch has 'emn, according to Documentation/process/coding-style.rst. > + return 0; > + > + return -ENOENT; > +} > +EXPORT_SYMBOL_GPL(rom_read_file); > + > /** > * find_reset_string - find the offset to the ``"RESET"`` string, if it exists > * @rom: ROM to search in MBR, Sergei
Hello! On 09/02/2019 12:05 PM, Sergei Shtylyov wrote: >> Reading ROM files is trivial since they are permanently available in >> memory. Having rom_read_file() is a convenient when for example >> resolving the machine region in subsequent changes. >> >> Signed-off-by: Fredrik Noring <noring@nocrew.org> > [...] >> index 12a57f24bd63..840d37a199d8 100644 >> --- a/arch/mips/ps2/rom.c >> +++ b/arch/mips/ps2/rom.c >> @@ -224,6 +224,39 @@ struct rom_file rom_first_file(const struct rom_dir dir) >> } >> EXPORT_SYMBOL_GPL(rom_first_file); >> +/** >> + * rom_read_file - read ROM file data >> + * @dir: directory to read the file from >> + * @name: file name to read >> + * @buffer: pointer to buffer to store data that is read >> + * @size: size in bytes to read >> + * @offset: offset in bytes to start reading >> + * >> + * Context: any >> + * Return: on successful completion, a nonnegative integer indicating the >> + * number of bytes actually read; otherwise, a negative error number >> + */ >> +ssize_t rom_read_file(const struct rom_dir dir, >> + const char *name, void *buffer, size_t size, loff_t offset) >> +{ >> + struct rom_file file; >> + >> + rom_find_files(file, dir, name) >> + if (offset < file.size) { >> + const u8 *b = file.data; >> + size_t remaining = file.size - offset; >> + size_t n = min(size, remaining); >> + >> + memcpy(buffer, &b[offset], n); >> + >> + return n; >> + } else > > The *else* branch also needs {} if the *if* branch has 'emn, according to Documentation/process/coding-style.rst. Just realized that we don't need *else* after *return*... >> + return 0; >> + >> + return -ENOENT; >> +} >> +EXPORT_SYMBOL_GPL(rom_read_file); >> + >> /** >> * find_reset_string - find the offset to the ``"RESET"`` string, if it exists >> * @rom: ROM to search in MBR, Sergei
> >> + struct rom_file file; > >> + > >> + rom_find_files(file, dir, name) > >> + if (offset < file.size) { > >> + const u8 *b = file.data; > >> + size_t remaining = file.size - offset; > >> + size_t n = min(size, remaining); > >> + > >> + memcpy(buffer, &b[offset], n); > >> + > >> + return n; > >> + } else > > > > The *else* branch also needs {} if the *if* branch has 'emn, according to Documentation/process/coding-style.rst. > > Just realized that we don't need *else* after *return*... > > >> + return 0; There is a style rule that says "use braces when a loop contains more than a single simple statement", and with those braces we could remove the else: rom_find_files (file, dir, name) { if (offset < file.size) { const u8 *b = file.data; size_t remaining = file.size - offset; size_t n = min(size, remaining); memcpy(buffer, &b[offset], n); return n; } return 0; } [ rom_find_files() is a cursor based loop macro similar to list_for_each_entry() and others of the same kind. Perhaps the name could be a bit better, for example rom_for_each_file_with_name() or something like that. Patches 036/120 and 037/120 have the details. ] Fredrik
diff --git a/arch/mips/include/asm/mach-ps2/rom.h b/arch/mips/include/asm/mach-ps2/rom.h index 8794bd13184e..f2c35788ddfb 100644 --- a/arch/mips/include/asm/mach-ps2/rom.h +++ b/arch/mips/include/asm/mach-ps2/rom.h @@ -98,6 +98,9 @@ extern struct rom_dir rom1_dir; /* ROM1 directory (DVD) */ rom_for_each_file((file), (dir)) \ if (strcmp((file).name, filename) != 0) continue; else +ssize_t rom_read_file(const struct rom_dir dir, + const char *name, void *buffer, size_t size, loff_t offset); + bool rom_empty_dir(const struct rom_dir dir); bool rom_terminating_file(const struct rom_file file); diff --git a/arch/mips/ps2/rom.c b/arch/mips/ps2/rom.c index 12a57f24bd63..840d37a199d8 100644 --- a/arch/mips/ps2/rom.c +++ b/arch/mips/ps2/rom.c @@ -224,6 +224,39 @@ struct rom_file rom_first_file(const struct rom_dir dir) } EXPORT_SYMBOL_GPL(rom_first_file); +/** + * rom_read_file - read ROM file data + * @dir: directory to read the file from + * @name: file name to read + * @buffer: pointer to buffer to store data that is read + * @size: size in bytes to read + * @offset: offset in bytes to start reading + * + * Context: any + * Return: on successful completion, a nonnegative integer indicating the + * number of bytes actually read; otherwise, a negative error number + */ +ssize_t rom_read_file(const struct rom_dir dir, + const char *name, void *buffer, size_t size, loff_t offset) +{ + struct rom_file file; + + rom_find_files(file, dir, name) + if (offset < file.size) { + const u8 *b = file.data; + size_t remaining = file.size - offset; + size_t n = min(size, remaining); + + memcpy(buffer, &b[offset], n); + + return n; + } else + return 0; + + return -ENOENT; +} +EXPORT_SYMBOL_GPL(rom_read_file); + /** * find_reset_string - find the offset to the ``"RESET"`` string, if it exists * @rom: ROM to search in
Reading ROM files is trivial since they are permanently available in memory. Having rom_read_file() is a convenient when for example resolving the machine region in subsequent changes. Signed-off-by: Fredrik Noring <noring@nocrew.org> --- arch/mips/include/asm/mach-ps2/rom.h | 3 +++ arch/mips/ps2/rom.c | 33 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)