Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-25 Thread Vaishali Thakkar
On Mon, 25 Mar 2019 at 21:31, Stephen Boyd  wrote:
>
> Quoting Vaishali Thakkar (2019-03-24 10:42:36)
> > On Sat, 23 Mar 2019 at 05:31, Stephen Boyd  wrote:
> >
> > > debugfs_create_devm_seqfile() which may work to print a string from some
> > > struct member. I'm not sure why you're using simple_attr_read(). Where
> > > does that become important?
> >
> > DEFINE_DEBUGFS_ATTRIBUTE has simple_attr helpers which
> > expects int value. So, in the case of a string it requires to implement
> > similar macro and separate helpers for the same.
>
> Why does DEFINE_DEBUGFS_ATTRIBUTE need to be used?

For defining files inside the debugfs code [with get/set functions].
Also, most of the similar functions in the debugfs core seems to be
using it.

>


Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-25 Thread Stephen Boyd
Quoting Vaishali Thakkar (2019-03-24 10:42:36)
> On Sat, 23 Mar 2019 at 05:31, Stephen Boyd  wrote:
> 
> > debugfs_create_devm_seqfile() which may work to print a string from some
> > struct member. I'm not sure why you're using simple_attr_read(). Where
> > does that become important?
> 
> DEFINE_DEBUGFS_ATTRIBUTE has simple_attr helpers which
> expects int value. So, in the case of a string it requires to implement
> similar macro and separate helpers for the same.

Why does DEFINE_DEBUGFS_ATTRIBUTE need to be used?



Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-24 Thread Vaishali Thakkar
On Sat, 23 Mar 2019 at 05:31, Stephen Boyd  wrote:
>
> Quoting Vaishali Thakkar (2019-03-20 22:51:20)
> > On Thu, 14 Mar 2019 at 21:28, Stephen Boyd  wrote:
> > >
> > > Quoting Vaishali Thakkar (2019-03-14 04:25:16)
> > > > On Fri, 1 Mar 2019 at 03:02, Stephen Boyd  wrote:
> > >
> > > >
> > > > In the case of converting it to cpu native during probe, I'll need to
> > > > declare an extra struct with u32 being the parsed version for it to be
> > > > correct. Wouldn't it add extra overhead?
> > >
> > > Yes it would be some small extra overhead that could be allocated on the
> > > kernel's heap. What's the maximum size? A hundred bytes or so?
> > >
> > > I don't see much of a problem with this approach. It simplifies the
> > > patch series because nothing new is introduced in debugfs core and the
> > > endian conversion is done once in one place instead of being scattered
> > > throughout the code. Sounds like a good improvement to me.
> > >
> >
> > Yes, it's true that this approach is better than introducing new endian
> > functions in debugfs core but we should also keep in mind that this is
> > applicable only for 4 use cases. For other usecases, we want to print
> > string and hex values. So, I would either need new debugfs core
> > functions for the same. I tried introducing debugfs_create_str for string
> > values but we're ending up with introducing bunch of other helpers in
> > the core as simple_attr_read expects integer values. Similarly, for hex
> > values , I can't use debugfs_create_u32 as defined attributes in the
> > core has unsigned int as a specifier, will need to introduce some extra
> > helpers over there again.
>
> I imagine there are other uses of printing a string and hex value in
> debugfs. There's debugfs_create_x32() and debugfs_create_x64() for the
> hex value printing part (if you want that format). There's also

Ok.

> debugfs_create_devm_seqfile() which may work to print a string from some
> struct member. I'm not sure why you're using simple_attr_read(). Where
> does that become important?

DEFINE_DEBUGFS_ATTRIBUTE has simple_attr helpers which
expects int value. So, in the case of a string it requires to implement
similar macro and separate helpers for the same.

> >
> > Also, in case of keeping all other cases as it is, it'll look quite
> > asymmetric to use debugfs u32 function in init and using local macros
> > for other cases. I can have DEBUGFS_UINT_ADD like wrapper
> > macro for debugfs_create_u32 but again not sure if doing
> > all of this looks better than what we have at the moment as just having
> > 3 local macros covering our all cases without having lot of duplicated
> > code.
> >
> >  Let me know if about your opinion on the same. Thanks.
>
> My opinion is still that it would be best to push things that aren't SoC
> specific into the debugfs core and try to use as much from the core as
> possible. There doesn't seem to be anything very SoC specific here so
> I'm lost why this isn't doable.

Yes, that's true. We don't have much of SoC specific code here.

>


Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-22 Thread Stephen Boyd
Quoting Vaishali Thakkar (2019-03-20 22:51:20)
> On Thu, 14 Mar 2019 at 21:28, Stephen Boyd  wrote:
> >
> > Quoting Vaishali Thakkar (2019-03-14 04:25:16)
> > > On Fri, 1 Mar 2019 at 03:02, Stephen Boyd  wrote:
> >
> > >
> > > In the case of converting it to cpu native during probe, I'll need to
> > > declare an extra struct with u32 being the parsed version for it to be
> > > correct. Wouldn't it add extra overhead?
> >
> > Yes it would be some small extra overhead that could be allocated on the
> > kernel's heap. What's the maximum size? A hundred bytes or so?
> >
> > I don't see much of a problem with this approach. It simplifies the
> > patch series because nothing new is introduced in debugfs core and the
> > endian conversion is done once in one place instead of being scattered
> > throughout the code. Sounds like a good improvement to me.
> >
> 
> Yes, it's true that this approach is better than introducing new endian
> functions in debugfs core but we should also keep in mind that this is
> applicable only for 4 use cases. For other usecases, we want to print
> string and hex values. So, I would either need new debugfs core
> functions for the same. I tried introducing debugfs_create_str for string
> values but we're ending up with introducing bunch of other helpers in
> the core as simple_attr_read expects integer values. Similarly, for hex
> values , I can't use debugfs_create_u32 as defined attributes in the
> core has unsigned int as a specifier, will need to introduce some extra
> helpers over there again.

I imagine there are other uses of printing a string and hex value in
debugfs. There's debugfs_create_x32() and debugfs_create_x64() for the
hex value printing part (if you want that format). There's also
debugfs_create_devm_seqfile() which may work to print a string from some
struct member. I'm not sure why you're using simple_attr_read(). Where
does that become important?

> 
> Also, in case of keeping all other cases as it is, it'll look quite
> asymmetric to use debugfs u32 function in init and using local macros
> for other cases. I can have DEBUGFS_UINT_ADD like wrapper
> macro for debugfs_create_u32 but again not sure if doing
> all of this looks better than what we have at the moment as just having
> 3 local macros covering our all cases without having lot of duplicated
> code.
> 
>  Let me know if about your opinion on the same. Thanks.

My opinion is still that it would be best to push things that aren't SoC
specific into the debugfs core and try to use as much from the core as
possible. There doesn't seem to be anything very SoC specific here so
I'm lost why this isn't doable.



Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-20 Thread Vaishali Thakkar
On Thu, 14 Mar 2019 at 21:28, Stephen Boyd  wrote:
>
> Quoting Vaishali Thakkar (2019-03-14 04:25:16)
> > On Fri, 1 Mar 2019 at 03:02, Stephen Boyd  wrote:
> > >
> > > Why can't we use the debugfs_create_u32 function? It would make things
> > > clearer if there was either a debugfs_create_le32() function or if the
> > > socinfo structure stored in smem was unmarshalled from little endian
> > > to the cpu native endian format during probe time and then all the rest
> > > of the code can treat it as a native endian u32 values.
> >
> > Thanks for the review. I've worked on the next version with all the
> > changes you suggested in the patchset but I'm kind of not sure
> > what would be the best solution in this case.
>
> Alright, thanks.
>
> >
> > I'm bit skeptical about introducing debugfs_create_le32 as we don't
> > really have any endian specific functions in the debugfs core at the
> > moment. And if I do that, should I also introduce debugfs_create_be32
> > [and to an extent debugfs_create_le{16,64}]? More importantly, would
> > it be useful to introduce them in core?
>
> I suppose it's ambiguous if the endianness should be swapped to be CPU
> native, or if it should just export them as little endian or big endian
> to userspace. I wouldn't introduce any APIs that aren't used, because
> it's just dead code. If the code is documented clearly and indicates
> what it does then it should be fine. This patch has pretty much already
> written the code, so it's just a matter of moving it into debugfs core
> now and getting gregkh to approve.
>
> >
> > In the case of converting it to cpu native during probe, I'll need to
> > declare an extra struct with u32 being the parsed version for it to be
> > correct. Wouldn't it add extra overhead?
>
> Yes it would be some small extra overhead that could be allocated on the
> kernel's heap. What's the maximum size? A hundred bytes or so?
>
> I don't see much of a problem with this approach. It simplifies the
> patch series because nothing new is introduced in debugfs core and the
> endian conversion is done once in one place instead of being scattered
> throughout the code. Sounds like a good improvement to me.
>

Yes, it's true that this approach is better than introducing new endian
functions in debugfs core but we should also keep in mind that this is
applicable only for 4 use cases. For other usecases, we want to print
string and hex values. So, I would either need new debugfs core
functions for the same. I tried introducing debugfs_create_str for string
values but we're ending up with introducing bunch of other helpers in
the core as simple_attr_read expects integer values. Similarly, for hex
values , I can't use debugfs_create_u32 as defined attributes in the
core has unsigned int as a specifier, will need to introduce some extra
helpers over there again.

Also, in case of keeping all other cases as it is, it'll look quite
asymmetric to use debugfs u32 function in init and using local macros
for other cases. I can have DEBUGFS_UINT_ADD like wrapper
macro for debugfs_create_u32 but again not sure if doing
all of this looks better than what we have at the moment as just having
3 local macros covering our all cases without having lot of duplicated
code.

 Let me know if about your opinion on the same. Thanks.


Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-14 Thread Stephen Boyd
Quoting Vaishali Thakkar (2019-03-14 04:25:16)
> On Fri, 1 Mar 2019 at 03:02, Stephen Boyd  wrote:
> >
> > Why can't we use the debugfs_create_u32 function? It would make things
> > clearer if there was either a debugfs_create_le32() function or if the
> > socinfo structure stored in smem was unmarshalled from little endian
> > to the cpu native endian format during probe time and then all the rest
> > of the code can treat it as a native endian u32 values.
> 
> Thanks for the review. I've worked on the next version with all the
> changes you suggested in the patchset but I'm kind of not sure
> what would be the best solution in this case.

Alright, thanks.

> 
> I'm bit skeptical about introducing debugfs_create_le32 as we don't
> really have any endian specific functions in the debugfs core at the
> moment. And if I do that, should I also introduce debugfs_create_be32
> [and to an extent debugfs_create_le{16,64}]? More importantly, would
> it be useful to introduce them in core?

I suppose it's ambiguous if the endianness should be swapped to be CPU
native, or if it should just export them as little endian or big endian
to userspace. I wouldn't introduce any APIs that aren't used, because
it's just dead code. If the code is documented clearly and indicates
what it does then it should be fine. This patch has pretty much already
written the code, so it's just a matter of moving it into debugfs core
now and getting gregkh to approve.

> 
> In the case of converting it to cpu native during probe, I'll need to
> declare an extra struct with u32 being the parsed version for it to be
> correct. Wouldn't it add extra overhead?

Yes it would be some small extra overhead that could be allocated on the
kernel's heap. What's the maximum size? A hundred bytes or so?

I don't see much of a problem with this approach. It simplifies the
patch series because nothing new is introduced in debugfs core and the
endian conversion is done once in one place instead of being scattered
throughout the code. Sounds like a good improvement to me.



Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-14 Thread Vaishali Thakkar
On Fri, 1 Mar 2019 at 03:02, Stephen Boyd  wrote:
>
> Quoting Vaishali Thakkar (2019-02-24 22:50:43)
> > diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> > index 02078049fac7..ccadeba69a81 100644
> > --- a/drivers/soc/qcom/socinfo.c
> > +++ b/drivers/soc/qcom/socinfo.c
> > @@ -70,6 +93,10 @@ struct socinfo {
> >  struct qcom_socinfo {
> > struct soc_device *soc_dev;
> > struct soc_device_attribute attr;
> > +#ifdef CONFIG_DEBUG_FS
> > +   struct dentry *dbg_root;
> > +#endif /* CONFIG_DEBUG_FS */
> > +   struct socinfo *socinfo;
>
> This doesn't look necessary, instead just pass it through to the
> functions that need the pointer.
>
> >  };
> >
> >  struct soc_of_id {
> > @@ -133,6 +160,171 @@ static const char *socinfo_machine(struct device 
> > *dev, unsigned int id)
> > return NULL;
> >  }
> >
> > +#ifdef CONFIG_DEBUG_FS
> > +
> > +#define UINT_SHOW(name, attr)  \
> > +static int qcom_show_##name(struct seq_file *seq, void *p) \
> > +{  \
> > +   struct socinfo *socinfo = seq->private; \
> > +   seq_printf(seq, "%u\n", le32_to_cpu(socinfo->attr));\
> > +   return 0;   \
> > +}  \
> > +static int qcom_open_##name(struct inode *inode, struct file *file)\
> > +{  \
> > +   return single_open(file, qcom_show_##name, inode->i_private);   \
> > +}  \
> > +   \
> > +static const struct file_operations qcom_ ##name## _ops = {\
> > +   .open = qcom_open_##name,   \
> > +   .read = seq_read,   \
> > +   .llseek = seq_lseek,\
> > +   .release = single_release,  \
> > +}
>
> Why can't we use the debugfs_create_u32 function? It would make things
> clearer if there was either a debugfs_create_le32() function or if the
> socinfo structure stored in smem was unmarshalled from little endian
> to the cpu native endian format during probe time and then all the rest
> of the code can treat it as a native endian u32 values.

Thanks for the review. I've worked on the next version with all the
changes you suggested in the patchset but I'm kind of not sure
what would be the best solution in this case.

I'm bit skeptical about introducing debugfs_create_le32 as we don't
really have any endian specific functions in the debugfs core at the
moment. And if I do that, should I also introduce debugfs_create_be32
[and to an extent debugfs_create_le{16,64}]? More importantly, would
it be useful to introduce them in core?

In the case of converting it to cpu native during probe, I'll need to
declare an extra struct with u32 being the parsed version for it to be
correct. Wouldn't it add extra overhead?

> > +
> > +#define DEBUGFS_UINT_ADD(name) \
> > +   debugfs_create_file(__stringify(name), 0400,\
> > +   qcom_socinfo->dbg_root, \
> > +   qcom_socinfo->socinfo, _ ##name## _ops)
> > +
> > +#define HEX_SHOW(name, attr)   \
> > +static int qcom_show_##name(struct seq_file *seq, void *p) \
> > +{  \
> > +   struct socinfo *socinfo = seq->private; \
> > +   seq_printf(seq, "0x%x\n", le32_to_cpu(socinfo->attr));  \
>
> Use "%#x\n" format?
>
> > +   return 0;   \
> > +}  \
> > +static int qcom_open_##name(struct inode *inode, struct file *file)\
> > +{  \
> > +   return single_open(file, qcom_show_##name, inode->i_private);   \
> > +}  \
> > +   \
> > +static const struct file_operations qcom_ ##name## _ops = {\
> > +   .open = qcom_open_##name,   \
> > +   .read = seq_read,   \
> > +   .llseek = seq_lseek,\
> > +   .release = single_release,  \
> > +}
> > +
> > +#define DEBUGFS_HEX_ADD(name)   

Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-03-01 Thread Bjorn Andersson
On Sun 24 Feb 22:50 PST 2019, Vaishali Thakkar wrote:

> +#ifdef CONFIG_DEBUG_FS
> +/* pmic model info */

Please drop this comment and make "pmic_model" plural.

> +static const char *const pmic_model[] = {
> + [0]  = "Unknown PMIC model",
> + [9]  = "PM8994",
> + [11] = "PM8916",
> + [13] = "PM8058",
> + [14] = "PM8028",
> + [15] = "PM8901",
> + [16] = "PM8027",
> + [17] = "ISL9519",
> + [18] = "PM8921",
> + [19] = "PM8018",
> + [20] = "PM8015",
> + [21] = "PM8014",
> + [22] = "PM8821",
> + [23] = "PM8038",
> + [24] = "PM8922",
> + [25] = "PM8917",
> +};
> +#endif /* CONFIG_DEBUG_FS */
[..]
> +static int qcom_show_pmic_model(struct seq_file *seq, void *p)
> +{
> + struct socinfo *socinfo = seq->private;
> + int model = SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_model));
> +
> + if (model < 0)
> + return -EINVAL;

You need to deal with the fact that model might be >=
ARRAY_SIZE(pmic_model) and that pmic_mode[model] might be NULL, in the
event that you missed entries in the list or this driver is used on
newer platforms.

> +
> + seq_printf(seq, "%s\n", pmic_model[model]);
> +
> + return 0;
> +}
> +
> +static int qcom_show_pmic_die_revision(struct seq_file *seq, void *p)
> +{
> + struct socinfo *socinfo = seq->private;
> +
> + seq_printf(seq, "%u.%u\n",
> +SOCINFO_MAJOR(le32_to_cpu(socinfo->pmic_die_rev)),
> +SOCINFO_MINOR(le32_to_cpu(socinfo->pmic_die_rev)));
> +
> + return 0;
> +}
> +
> +UINT_SHOW(raw_version, raw_ver);
> +UINT_SHOW(hardware_platform, hw_plat);
> +UINT_SHOW(platform_version, plat_ver);
> +UINT_SHOW(foundry_id, foundry_id);
> +HEX_SHOW(chip_family, chip_family);
> +HEX_SHOW(raw_device_family, raw_device_family);
> +HEX_SHOW(raw_device_number, raw_device_num);
> +QCOM_OPEN(build_id, qcom_show_build_id);
> +QCOM_OPEN(accessory_chip, qcom_show_accessory_chip);
> +QCOM_OPEN(pmic_model, qcom_show_pmic_model);
> +QCOM_OPEN(platform_subtype, qcom_show_platform_subtype);
> +QCOM_OPEN(pmic_die_revision, qcom_show_pmic_die_revision);
> +
> +static void socinfo_debugfs_init(struct qcom_socinfo *qcom_socinfo)
> +{
> + qcom_socinfo->dbg_root = debugfs_create_dir("qcom_socinfo", NULL);
> +
> + DEBUGFS_UINT_ADD(raw_version);
> + DEBUGFS_UINT_ADD(hardware_platform);

Note that the content of struct socinfo has grown over time, so based on
the comments in the struct the size of the struct would not cover
hw_plat if version < 3.

So you should make the addition of these conditional on socinfo->ver.
As each version adds more entries I suggest that you do this with a:

switch (qcom_socinfo->socinfo->ver) {
case 12:
add v12 entries;
case 11:
add v11 entries;
case 10:
add v10 entries;
...
};

> + DEBUGFS_UINT_ADD(platform_version);
> + DEBUGFS_UINT_ADD(foundry_id);
> + DEBUGFS_HEX_ADD(chip_family);
> + DEBUGFS_HEX_ADD(raw_device_family);
> + DEBUGFS_HEX_ADD(raw_device_number);
> + DEBUGFS_ADD(build_id);
> + DEBUGFS_ADD(accessory_chip);
> + DEBUGFS_ADD(pmic_model);
> + DEBUGFS_ADD(platform_subtype);
> + DEBUGFS_ADD(pmic_die_revision);
> +}
> +

Regards,
Bjorn


Re: [PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-02-28 Thread Stephen Boyd
Quoting Vaishali Thakkar (2019-02-24 22:50:43)
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index 02078049fac7..ccadeba69a81 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -70,6 +93,10 @@ struct socinfo {
>  struct qcom_socinfo {
> struct soc_device *soc_dev;
> struct soc_device_attribute attr;
> +#ifdef CONFIG_DEBUG_FS
> +   struct dentry *dbg_root;
> +#endif /* CONFIG_DEBUG_FS */
> +   struct socinfo *socinfo;

This doesn't look necessary, instead just pass it through to the
functions that need the pointer.

>  };
>  
>  struct soc_of_id {
> @@ -133,6 +160,171 @@ static const char *socinfo_machine(struct device *dev, 
> unsigned int id)
> return NULL;
>  }
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +#define UINT_SHOW(name, attr)  \
> +static int qcom_show_##name(struct seq_file *seq, void *p) \
> +{  \
> +   struct socinfo *socinfo = seq->private; \
> +   seq_printf(seq, "%u\n", le32_to_cpu(socinfo->attr));\
> +   return 0;   \
> +}  \
> +static int qcom_open_##name(struct inode *inode, struct file *file)\
> +{  \
> +   return single_open(file, qcom_show_##name, inode->i_private);   \
> +}  \
> +   \
> +static const struct file_operations qcom_ ##name## _ops = {\
> +   .open = qcom_open_##name,   \
> +   .read = seq_read,   \
> +   .llseek = seq_lseek,\
> +   .release = single_release,  \
> +}

Why can't we use the debugfs_create_u32 function? It would make things
clearer if there was either a debugfs_create_le32() function or if the
socinfo structure stored in smem was unmarshalled from little endian
to the cpu native endian format during probe time and then all the rest
of the code can treat it as a native endian u32 values.

> +
> +#define DEBUGFS_UINT_ADD(name) \
> +   debugfs_create_file(__stringify(name), 0400,\
> +   qcom_socinfo->dbg_root, \
> +   qcom_socinfo->socinfo, _ ##name## _ops)
> +
> +#define HEX_SHOW(name, attr)   \
> +static int qcom_show_##name(struct seq_file *seq, void *p) \
> +{  \
> +   struct socinfo *socinfo = seq->private; \
> +   seq_printf(seq, "0x%x\n", le32_to_cpu(socinfo->attr));  \

Use "%#x\n" format?

> +   return 0;   \
> +}  \
> +static int qcom_open_##name(struct inode *inode, struct file *file)\
> +{  \
> +   return single_open(file, qcom_show_##name, inode->i_private);   \
> +}  \
> +   \
> +static const struct file_operations qcom_ ##name## _ops = {\
> +   .open = qcom_open_##name,   \
> +   .read = seq_read,   \
> +   .llseek = seq_lseek,\
> +   .release = single_release,  \
> +}
> +
> +#define DEBUGFS_HEX_ADD(name)  \
> +   debugfs_create_file(__stringify(name), 0400,\
> +   qcom_socinfo->dbg_root, \
> +   qcom_socinfo->socinfo, _ ##name## _ops)
> +
> +
> +#define QCOM_OPEN(name, _func) \
> +static int qcom_open_##name(struct inode *inode, struct file *file)\
> +{  \
> +   return single_open(file, _func, inode->i_private);  \
> +}  \
> +   \
> +static const struct file_operations qcom_ ##name## _ops = {\
> +   .open = qcom_open_##name,   \
> +   .read = 

[PATCH v4 4/5] soc: qcom: socinfo: Expose custom attributes

2019-02-24 Thread Vaishali Thakkar
The Qualcomm socinfo provides a number of additional attributes,
add these to the socinfo driver and expose them via debugfs
functionality.

Signed-off-by: Vaishali Thakkar 
---
Changes since v3:
- Fix compilation error in function signatures when
  debugfs is disabled
Changes since v2:
- None
Changes since v1:
- Remove unnecessary debugfs dir creation check
- Align ifdefs to left
- Fix function signatures for debugfs init/exit
---
 drivers/soc/qcom/socinfo.c | 198 +
 1 file changed, 198 insertions(+)

diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 02078049fac7..ccadeba69a81 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2017-2019, Linaro Ltd.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -29,6 +30,28 @@
  */
 #define SMEM_HW_SW_BUILD_ID137
 
+#ifdef CONFIG_DEBUG_FS
+/* pmic model info */
+static const char *const pmic_model[] = {
+   [0]  = "Unknown PMIC model",
+   [9]  = "PM8994",
+   [11] = "PM8916",
+   [13] = "PM8058",
+   [14] = "PM8028",
+   [15] = "PM8901",
+   [16] = "PM8027",
+   [17] = "ISL9519",
+   [18] = "PM8921",
+   [19] = "PM8018",
+   [20] = "PM8015",
+   [21] = "PM8014",
+   [22] = "PM8821",
+   [23] = "PM8038",
+   [24] = "PM8922",
+   [25] = "PM8917",
+};
+#endif /* CONFIG_DEBUG_FS */
+
 /* Socinfo SMEM item structure */
 struct socinfo {
__le32 fmt;
@@ -70,6 +93,10 @@ struct socinfo {
 struct qcom_socinfo {
struct soc_device *soc_dev;
struct soc_device_attribute attr;
+#ifdef CONFIG_DEBUG_FS
+   struct dentry *dbg_root;
+#endif /* CONFIG_DEBUG_FS */
+   struct socinfo *socinfo;
 };
 
 struct soc_of_id {
@@ -133,6 +160,171 @@ static const char *socinfo_machine(struct device *dev, 
unsigned int id)
return NULL;
 }
 
+#ifdef CONFIG_DEBUG_FS
+
+#define UINT_SHOW(name, attr)  \
+static int qcom_show_##name(struct seq_file *seq, void *p) \
+{  \
+   struct socinfo *socinfo = seq->private; \
+   seq_printf(seq, "%u\n", le32_to_cpu(socinfo->attr));\
+   return 0;   \
+}  \
+static int qcom_open_##name(struct inode *inode, struct file *file)\
+{  \
+   return single_open(file, qcom_show_##name, inode->i_private);   \
+}  \
+   \
+static const struct file_operations qcom_ ##name## _ops = {\
+   .open = qcom_open_##name,   \
+   .read = seq_read,   \
+   .llseek = seq_lseek,\
+   .release = single_release,  \
+}
+
+#define DEBUGFS_UINT_ADD(name) \
+   debugfs_create_file(__stringify(name), 0400,\
+   qcom_socinfo->dbg_root, \
+   qcom_socinfo->socinfo, _ ##name## _ops)
+
+#define HEX_SHOW(name, attr)   \
+static int qcom_show_##name(struct seq_file *seq, void *p) \
+{  \
+   struct socinfo *socinfo = seq->private; \
+   seq_printf(seq, "0x%x\n", le32_to_cpu(socinfo->attr));  \
+   return 0;   \
+}  \
+static int qcom_open_##name(struct inode *inode, struct file *file)\
+{  \
+   return single_open(file, qcom_show_##name, inode->i_private);   \
+}  \
+   \
+static const struct file_operations qcom_ ##name## _ops = {\
+   .open = qcom_open_##name,   \
+   .read = seq_read,   \
+   .llseek = seq_lseek,\
+   .release = single_release,  \
+}
+
+#define DEBUGFS_HEX_ADD(name)  \
+   debugfs_create_file(__stringify(name), 0400,\
+