On 21/03/2022 17:02, Greg Kroah-Hartman wrote:
On Mon, Mar 21, 2022 at 04:46:50PM +0700, Quan Nguyen wrote:
On 21/03/2022 15:21, Greg Kroah-Hartman wrote:
On Mon, Mar 21, 2022 at 03:13:51PM +0700, Quan Nguyen wrote:
This commit adds driver support for accessing various information
reported by Ampere's SMpro co-processor such as Boot Progress and
other miscellaneous data.
Signed-off-by: Quan Nguyen <quan@xxxxxxxxxxxxxxxxxxxxxx>
No Documentation/ABI/ entries for your sysfs file?
Thank you, Greg, for a very quick review.
I have put this file in other patch.
+static ssize_t boot_progress_show(struct device *dev, struct device_attribute *da, char *buf)
+{
+ struct smpro_misc *misc = dev_get_drvdata(dev);
+ u32 boot_progress;
+ u8 current_stage;
+ u8 boot_status;
+ u8 boot_stage;
+ u32 select;
+ u32 reg_lo;
+ u32 reg;
+ int ret;
+
+ /* Read current boot stage */
+ ret = regmap_read(misc->regmap, BOOTSTAGE_CUR_STAGE, ®);
+ if (ret)
+ return ret;
+
+ current_stage = reg & 0xff;
+
+ /* Read the boot progress */
+ ret = regmap_read(misc->regmap, BOOTSTAGE_SELECT, &select);
+ if (ret)
+ return ret;
+
+ boot_stage = (select >> 8) & 0xff;
+ boot_status = select & 0xff;
+
+ if (boot_stage > current_stage)
+ return -EINVAL;
+
+ ret = regmap_read(misc->regmap, BOOTSTAGE_STATUS_LO, ®_lo);
+ if (!ret)
+ ret = regmap_read(misc->regmap, BOOTSTAGE_STATUS_HI, ®);
+ if (ret)
+ return ret;
+
+ boot_progress = swab16(reg) << 16 | swab16(reg_lo);
+
+ /* Tell firmware to provide next boot stage next time */
+ if (boot_stage < current_stage) {
+ ret = regmap_write(misc->regmap, BOOTSTAGE_SELECT, ((select & 0xff00) | 0x1));
+ if (ret)
+ return ret;
+ }
+
+ return snprintf(buf, PAGE_SIZE, "0x%02x 0x%02x 0x%08x\n",
+ boot_stage, boot_status, boot_progress);
sysfs_emit() please.
Thanks, Greg.
Will switch to sysfs_emit() in my next version.
Also, this is 3 different things, put all of these in different sysfs
files.
thanks,
greg k-h
Actually, no. It is single value of boot stage.
You are displaying 3 things in a single line, those are 3 different
things.
Let me explain:
The boot progress consists of three things together: boot_stage, boot_status
and boot_progress and they have no meaning if reported them as three
separate values:
+ boot_stage is to indicate the boot stage
+ boot_status is to report the result of that boot_stage: started, complete
or error.
+ boot_progress is to report more extra information for the stage other than
the boot_stage/boot_status.
Why are these just not 3 different files? They describe three different
things, please do not EVER force userspace to parse a sysfs file other
than a single value.
Thanks Greg for the comment.
As there are multiple boot stages that occur even before the firmware is
able to report, the firmware will stores all the stages and make them
ready for later access. Later, on each access, ie: read to sysfs, the
earliest boot stages will be read out and the next stage info is made
ready for the next read by clearing the current reported stage.
As these three piece of info only make sense when put together, we chose
to providing a single file only so that when the sysfs is read, we can
provide them as a single value and do the clearing so that the next read
will return with the next stage.
My intention for next version is to report a single 12-byte hex value
for this sysfs.
- Quan
