Re: [RFC PATCH 4/4] ultrasoc: Add System Memory Buffer driver

[liuqi (BA)]

Hi Suzuki,
On 2021/6/25 6:47, Suzuki K Poulose wrote:
> Hi Qi
>
> On 15/06/2021 10:34, Qi Liu wrote:
> > This patch adds driver for System Memory Buffer. It includes
> > a platform driver for the SMB device.
> >
> > Signed-off-by: Jonathan Zhou <jonathan.zhouwen@xxxxxxxxxx>
> > Signed-off-by: Qi Liu <liuqi115@xxxxxxxxxx>
> > ---
> >   drivers/hwtracing/ultrasoc/Kconfig        |   9 +
> >   drivers/hwtracing/ultrasoc/Makefile       |   3 +
> >   drivers/hwtracing/ultrasoc/ultrasoc-smb.c | 663 
> > ++++++++++++++++++++++++++++++
> >   drivers/hwtracing/ultrasoc/ultrasoc-smb.h | 182 ++++++++
> >   4 files changed, 857 insertions(+)
> >   create mode 100644 drivers/hwtracing/ultrasoc/ultrasoc-smb.c
> >   create mode 100644 drivers/hwtracing/ultrasoc/ultrasoc-smb.h
>
> > +/*
> > + * Coresight doesn't export the following
> > + * structures(cs_mode,cs_buffers,etm_event_data),
> > + * so we redefine a copy here.
> > + */
>
> Please do not duplicate them. This indicates, either :
>
>   - You need to place your driver under coresight
>
>   OR
>
>   - Export the required definitions.

got it, I'll move this driver to coresight/ultrasoc, thanks.

Qi
> > +enum cs_mode {
> > +    CS_MODE_DISABLED,
> > +    CS_MODE_SYSFS,
> > +    CS_MODE_PERF,
> > +};
> > +
>
> > +struct cs_buffers {
> > +    unsigned int        cur;
> > +    unsigned int        nr_pages;
> > +    unsigned long        offset;
> > +    local_t            data_size;
> > +    bool            snapshot;
> > +    void            **data_pages;
> > +};
> > +
>
> Why does this need to be replicated ?
>
> > +struct etm_event_data {
> > +    struct work_struct work;
> > +    cpumask_t mask;
> > +    void *snk_config;
> > +    struct list_head * __percpu *path;
> > +};
> > +
> > +#if IS_ENABLED(CONFIG_CORESIGHT)
> > +int etm_perf_symlink(struct coresight_device *csdev, bool link);
> > +int etm_perf_add_symlink_sink(struct coresight_device *csdev);
> > +void etm_perf_del_symlink_sink(struct coresight_device *csdev);
> > +static inline void *etm_perf_sink_config(struct perf_output_handle 
> > *handle)
> > +{
> > +    struct etm_event_data *data = perf_get_aux(handle);
> > +
> > +    if (data)
> > +        return data->snk_config;
> > +    return NULL;
> > +}
> > +#else
> > +static inline int etm_perf_symlink(struct coresight_device *csdev, 
> > bool link)
> > +{ return -EINVAL; }
> > +int etm_perf_add_symlink_sink(struct coresight_device *csdev)
> > +{ return -EINVAL; }
> > +void etm_perf_del_symlink_sink(struct coresight_device *csdev) {}
> > +static inline void *etm_perf_sink_config(struct perf_output_handle 
> > *handle)
> > +{
> > +    return NULL;
> > +}
> > +
> > +#endif /* CONFIG_CORESIGHT */
> > +
> > +#endif
>
> Suzuki
> .