* [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
@ 2023-08-02 9:57 Tzung-Bi Shih
2023-08-02 15:15 ` Muhammad Usama Anjum
2023-08-02 16:04 ` Guenter Roeck
0 siblings, 2 replies; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-08-02 9:57 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum, briannorris
`element->buffer.pointer` should be binary blob. `%s` doesn't work
perfect for them.
Print hex string for ACPI_TYPE_BUFFER.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
Changes from v1[1]:
- Use hex_dump_to_buffer().
- Rewrite the loop for catching edge cases for truncating.
[1]: https://patchwork.kernel.org/project/chrome-platform/patch/20230726073127.2969387-1-tzungbi@kernel.org/
drivers/platform/chrome/chromeos_acpi.c | 31 ++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/chromeos_acpi.c b/drivers/platform/chrome/chromeos_acpi.c
index 50d8a4d4352d..761506d307ad 100644
--- a/drivers/platform/chrome/chromeos_acpi.c
+++ b/drivers/platform/chrome/chromeos_acpi.c
@@ -90,7 +90,36 @@ static int chromeos_acpi_handle_package(struct device *dev, union acpi_object *o
case ACPI_TYPE_STRING:
return sysfs_emit(buf, "%s\n", element->string.pointer);
case ACPI_TYPE_BUFFER:
- return sysfs_emit(buf, "%s\n", element->buffer.pointer);
+ {
+ int i, r, at, room_left;
+ const int byte_per_line = 16;
+
+ at = 0;
+ room_left = PAGE_SIZE - 1;
+ for (i = 0; i < element->buffer.length && room_left; i += byte_per_line) {
+ r = hex_dump_to_buffer(element->buffer.pointer + i,
+ element->buffer.length - i,
+ byte_per_line, 1, buf + at, room_left,
+ false);
+ if (r > room_left)
+ goto truncating;
+ at += r;
+ room_left -= r;
+
+ r = sysfs_emit_at(buf, at, "\n");
+ if (!r)
+ goto truncating;
+ at += r;
+ room_left -= r;
+ }
+
+ buf[at] = 0;
+ return at;
+truncating:
+ dev_info(dev, "truncating sysfs content for %s\n", name);
+ sysfs_emit_at(buf, PAGE_SIZE - 4, "..\n");
+ return PAGE_SIZE - 1;
+ }
default:
dev_err(dev, "element type %d not supported\n", element->type);
return -EINVAL;
--
2.41.0.585.gd2178a4bd4-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 9:57 [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER Tzung-Bi Shih
@ 2023-08-02 15:15 ` Muhammad Usama Anjum
2023-08-03 1:23 ` Tzung-Bi Shih
2023-08-02 16:04 ` Guenter Roeck
1 sibling, 1 reply; 8+ messages in thread
From: Muhammad Usama Anjum @ 2023-08-02 15:15 UTC (permalink / raw)
To: Tzung-Bi Shih, bleung, groeck
Cc: Muhammad Usama Anjum, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, briannorris
On 8/2/23 2:57 PM, Tzung-Bi Shih wrote:
> `element->buffer.pointer` should be binary blob. `%s` doesn't work
> perfect for them.
>
> Print hex string for ACPI_TYPE_BUFFER.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Please add fixes tag and CC stable as well (stable@vger.kernel.org) in next
revision.
Fixes: 0a4cad9c11ad ("platform/chrome: Add ChromeOS ACPI device driver")
> ---
> Changes from v1[1]:
> - Use hex_dump_to_buffer().
> - Rewrite the loop for catching edge cases for truncating.
>
> [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20230726073127.2969387-1-tzungbi@kernel.org/
>
> drivers/platform/chrome/chromeos_acpi.c | 31 ++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/chromeos_acpi.c b/drivers/platform/chrome/chromeos_acpi.c
> index 50d8a4d4352d..761506d307ad 100644
> --- a/drivers/platform/chrome/chromeos_acpi.c
> +++ b/drivers/platform/chrome/chromeos_acpi.c
> @@ -90,7 +90,36 @@ static int chromeos_acpi_handle_package(struct device *dev, union acpi_object *o
> case ACPI_TYPE_STRING:
> return sysfs_emit(buf, "%s\n", element->string.pointer);
> case ACPI_TYPE_BUFFER:
> - return sysfs_emit(buf, "%s\n", element->buffer.pointer);
> + {
> + int i, r, at, room_left;
> + const int byte_per_line = 16;
> +
> + at = 0;
> + room_left = PAGE_SIZE - 1;
> + for (i = 0; i < element->buffer.length && room_left; i += byte_per_line) {
> + r = hex_dump_to_buffer(element->buffer.pointer + i,
> + element->buffer.length - i,
> + byte_per_line, 1, buf + at, room_left,
> + false);
> + if (r > room_left)
> + goto truncating;
> + at += r;
> + room_left -= r;
> +
> + r = sysfs_emit_at(buf, at, "\n");
> + if (!r)
> + goto truncating;
> + at += r;
> + room_left -= r;
> + }
> +
> + buf[at] = 0;
> + return at;
> +truncating:
> + dev_info(dev, "truncating sysfs content for %s\n", name);
> + sysfs_emit_at(buf, PAGE_SIZE - 4, "..\n");
> + return PAGE_SIZE - 1;
> + }
> default:
> dev_err(dev, "element type %d not supported\n", element->type);
> return -EINVAL;
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 15:15 ` Muhammad Usama Anjum
@ 2023-08-03 1:23 ` Tzung-Bi Shih
2023-08-03 4:17 ` Muhammad Usama Anjum
0 siblings, 1 reply; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-08-03 1:23 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, briannorris
On Wed, Aug 02, 2023 at 08:15:31PM +0500, Muhammad Usama Anjum wrote:
> On 8/2/23 2:57 PM, Tzung-Bi Shih wrote:
> > `element->buffer.pointer` should be binary blob. `%s` doesn't work
> > perfect for them.
> >
> > Print hex string for ACPI_TYPE_BUFFER.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> Please add fixes tag and CC stable as well (stable@vger.kernel.org) in next
> revision.
>
> Fixes: 0a4cad9c11ad ("platform/chrome: Add ChromeOS ACPI device driver")
I think a Fixes tag should be sufficient.
Fix in v3 (https://patchwork.kernel.org/project/chrome-platform/patch/20230803011245.3773756-1-tzungbi@kernel.org/).
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-03 1:23 ` Tzung-Bi Shih
@ 2023-08-03 4:17 ` Muhammad Usama Anjum
2023-08-07 3:37 ` Tzung-Bi Shih
0 siblings, 1 reply; 8+ messages in thread
From: Muhammad Usama Anjum @ 2023-08-03 4:17 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Muhammad Usama Anjum, bleung, groeck, chrome-platform,
guillaume.tucker, denys.f, ricardo.canuelo, briannorris
On 8/3/23 6:23 AM, Tzung-Bi Shih wrote:
> On Wed, Aug 02, 2023 at 08:15:31PM +0500, Muhammad Usama Anjum wrote:
>> On 8/2/23 2:57 PM, Tzung-Bi Shih wrote:
>>> `element->buffer.pointer` should be binary blob. `%s` doesn't work
>>> perfect for them.
>>>
>>> Print hex string for ACPI_TYPE_BUFFER.
>>>
>>> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
>> Please add fixes tag and CC stable as well (stable@vger.kernel.org) in next
>> revision.
>>
>> Fixes: 0a4cad9c11ad ("platform/chrome: Add ChromeOS ACPI device driver")
>
> I think a Fixes tag should be sufficient.
Why do you think so? As this is a bug fix which exists in the driver since
it was written, if we add fixes tag and probably cc stable as well, the
patch will be applied to all LTS kernels which we really want. In this way,
the bug would be fixed on all LTS kernels not just the latest kernel.
>
> Fix in v3 (https://patchwork.kernel.org/project/chrome-platform/patch/20230803011245.3773756-1-tzungbi@kernel.org/).
--
BR,
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-03 4:17 ` Muhammad Usama Anjum
@ 2023-08-07 3:37 ` Tzung-Bi Shih
2023-08-10 3:06 ` Tzung-Bi Shih
0 siblings, 1 reply; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-08-07 3:37 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, briannorris
On Thu, Aug 03, 2023 at 09:17:23AM +0500, Muhammad Usama Anjum wrote:
> On 8/3/23 6:23 AM, Tzung-Bi Shih wrote:
> > On Wed, Aug 02, 2023 at 08:15:31PM +0500, Muhammad Usama Anjum wrote:
> >> On 8/2/23 2:57 PM, Tzung-Bi Shih wrote:
> >>> `element->buffer.pointer` should be binary blob. `%s` doesn't work
> >>> perfect for them.
> >>>
> >>> Print hex string for ACPI_TYPE_BUFFER.
> >>>
> >>> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> >> Please add fixes tag and CC stable as well (stable@vger.kernel.org) in next
> >> revision.
> >>
> >> Fixes: 0a4cad9c11ad ("platform/chrome: Add ChromeOS ACPI device driver")
> >
> > I think a Fixes tag should be sufficient.
> Why do you think so? As this is a bug fix which exists in the driver since
> it was written, if we add fixes tag and probably cc stable as well, the
> patch will be applied to all LTS kernels which we really want. In this way,
> the bug would be fixed on all LTS kernels not just the latest kernel.
Without the patch, I observed some malfunctions of ChromiumOS userland program
`crossystem`. There is no kernel crashes and the system is bootable. Thus, I
think a Fixes tag should be sufficient.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-07 3:37 ` Tzung-Bi Shih
@ 2023-08-10 3:06 ` Tzung-Bi Shih
0 siblings, 0 replies; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-08-10 3:06 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, briannorris
On Mon, Aug 07, 2023 at 11:37:26AM +0800, Tzung-Bi Shih wrote:
> On Thu, Aug 03, 2023 at 09:17:23AM +0500, Muhammad Usama Anjum wrote:
> > On 8/3/23 6:23 AM, Tzung-Bi Shih wrote:
> > > On Wed, Aug 02, 2023 at 08:15:31PM +0500, Muhammad Usama Anjum wrote:
> > >> On 8/2/23 2:57 PM, Tzung-Bi Shih wrote:
> > >>> `element->buffer.pointer` should be binary blob. `%s` doesn't work
> > >>> perfect for them.
> > >>>
> > >>> Print hex string for ACPI_TYPE_BUFFER.
> > >>>
> > >>> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> > >> Please add fixes tag and CC stable as well (stable@vger.kernel.org) in next
> > >> revision.
> > >>
> > >> Fixes: 0a4cad9c11ad ("platform/chrome: Add ChromeOS ACPI device driver")
> > >
> > > I think a Fixes tag should be sufficient.
> > Why do you think so? As this is a bug fix which exists in the driver since
> > it was written, if we add fixes tag and probably cc stable as well, the
> > patch will be applied to all LTS kernels which we really want. In this way,
> > the bug would be fixed on all LTS kernels not just the latest kernel.
>
> Without the patch, I observed some malfunctions of ChromiumOS userland program
> `crossystem`. There is no kernel crashes and the system is bootable. Thus, I
> think a Fixes tag should be sufficient.
Let's cc stable as well.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 9:57 [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER Tzung-Bi Shih
2023-08-02 15:15 ` Muhammad Usama Anjum
@ 2023-08-02 16:04 ` Guenter Roeck
2023-08-03 1:20 ` Tzung-Bi Shih
1 sibling, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2023-08-02 16:04 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum, briannorris
On Wed, Aug 2, 2023 at 2:58 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> `element->buffer.pointer` should be binary blob. `%s` doesn't work
> perfect for them.
>
> Print hex string for ACPI_TYPE_BUFFER.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> Changes from v1[1]:
> - Use hex_dump_to_buffer().
> - Rewrite the loop for catching edge cases for truncating.
>
> [1]: https://patchwork.kernel.org/project/chrome-platform/patch/20230726073127.2969387-1-tzungbi@kernel.org/
>
> drivers/platform/chrome/chromeos_acpi.c | 31 ++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/chromeos_acpi.c b/drivers/platform/chrome/chromeos_acpi.c
> index 50d8a4d4352d..761506d307ad 100644
> --- a/drivers/platform/chrome/chromeos_acpi.c
> +++ b/drivers/platform/chrome/chromeos_acpi.c
> @@ -90,7 +90,36 @@ static int chromeos_acpi_handle_package(struct device *dev, union acpi_object *o
> case ACPI_TYPE_STRING:
> return sysfs_emit(buf, "%s\n", element->string.pointer);
> case ACPI_TYPE_BUFFER:
> - return sysfs_emit(buf, "%s\n", element->buffer.pointer);
> + {
> + int i, r, at, room_left;
> + const int byte_per_line = 16;
> +
> + at = 0;
> + room_left = PAGE_SIZE - 1;
> + for (i = 0; i < element->buffer.length && room_left; i += byte_per_line) {
> + r = hex_dump_to_buffer(element->buffer.pointer + i,
> + element->buffer.length - i,
> + byte_per_line, 1, buf + at, room_left,
> + false);
> + if (r > room_left)
> + goto truncating;
> + at += r;
> + room_left -= r;
> +
> + r = sysfs_emit_at(buf, at, "\n");
> + if (!r)
> + goto truncating;
> + at += r;
> + room_left -= r;
> + }
> +
> + buf[at] = 0;
> + return at;
> +truncating:
> + dev_info(dev, "truncating sysfs content for %s\n", name);
I really dislike that because it will, if it happens, clog the log
(the condition will be permanent). How about using dev_info_once() ?
Thanks,
Guenter
> + sysfs_emit_at(buf, PAGE_SIZE - 4, "..\n");
> + return PAGE_SIZE - 1;
> + }
> default:
> dev_err(dev, "element type %d not supported\n", element->type);
> return -EINVAL;
> --
> 2.41.0.585.gd2178a4bd4-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER
2023-08-02 16:04 ` Guenter Roeck
@ 2023-08-03 1:20 ` Tzung-Bi Shih
0 siblings, 0 replies; 8+ messages in thread
From: Tzung-Bi Shih @ 2023-08-03 1:20 UTC (permalink / raw)
To: Guenter Roeck
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f,
ricardo.canuelo, usama.anjum, briannorris
On Wed, Aug 02, 2023 at 09:04:10AM -0700, Guenter Roeck wrote:
> On Wed, Aug 2, 2023 at 2:58 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> > +truncating:
> > + dev_info(dev, "truncating sysfs content for %s\n", name);
>
> I really dislike that because it will, if it happens, clog the log
> (the condition will be permanent). How about using dev_info_once() ?
Fix in v3(https://patchwork.kernel.org/project/chrome-platform/patch/20230803011245.3773756-1-tzungbi@kernel.org/).
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-10 3:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-02 9:57 [PATCH v2] platform/chrome: chromeos_acpi: print hex string for ACPI_TYPE_BUFFER Tzung-Bi Shih
2023-08-02 15:15 ` Muhammad Usama Anjum
2023-08-03 1:23 ` Tzung-Bi Shih
2023-08-03 4:17 ` Muhammad Usama Anjum
2023-08-07 3:37 ` Tzung-Bi Shih
2023-08-10 3:06 ` Tzung-Bi Shih
2023-08-02 16:04 ` Guenter Roeck
2023-08-03 1:20 ` Tzung-Bi Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox