Re: [PATCH v1 2/3] tty/serial: ttvys: add null modem driver emulating serial port

[rishi gupta]

One problem (: about configfs. Desktop distros like Ubuntu doesn't
enable configFS by default in kernel config. So users will have to
build their own version of kernel. For my own purpose, as developer I
have control over this but for others building kernel suggested ?

On Fri, Jan 10, 2020 at 12:50 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Jan 09, 2020 at 02:59:59PM +0530, rishi gupta wrote:
> > > > +/* UART frame structure definitions */
> > > > +#define VS_CRTSCTS       0x0001
> > > > +#define VS_XON           0x0002
> > > > +#define VS_NONE          0X0004
> > > > +#define VS_DATA_5        0X0008
> > > > +#define VS_DATA_6        0X0010
> > > > +#define VS_DATA_7        0X0020
> > > > +#define VS_DATA_8        0X0040
> > >
> > > Why the "X"?
> > Sorry I did not understand, do you mean why VS_XON.
>
> No, I mean why the "0X0040" instead of "0x0040" like all other hex
> digits in your list of defines.
Okay understood.
>
> > > > +static int vs_alloc_reg_one_dev(int oidx, int pidx, int rtsmap,
> > > > +                     int dtrmap, int dtropn)
> > > > +{
> > > > +     int ret;
> > > > +     struct vs_dev *vsdev;
> > > > +     struct device *dev;
> > > > +
> > > > +     /* Allocate and init virtual tty device private data */
> > > > +     vsdev = kcalloc(1, sizeof(struct vs_dev), GFP_KERNEL);
> > > > +     if (!vsdev)
> > > > +             return -ENOMEM;
> > > > +
> > > > +     vsdev->own_tty = NULL;
> > > > +     vsdev->peer_tty = NULL;
> > > > +     vsdev->own_index = oidx;
> > > > +     vsdev->peer_index =  pidx;
> > > > +     vsdev->rts_mappings = rtsmap;
> > > > +     vsdev->dtr_mappings = dtrmap;
> > > > +     vsdev->set_odtr_at_open = dtropn;
> > > > +     vsdev->msr_reg = 0;
> > > > +     vsdev->mcr_reg = 0;
> > > > +     vsdev->waiting_msr_chg = 0;
> > > > +     vsdev->tx_paused = 0;
> > > > +     vsdev->faulty_cable = 0;
> > > > +     mutex_init(&vsdev->lock);
> > > > +
> > > > +     /* Register with tty core with specific minor number */
> > > > +     dev = tty_register_device(ttyvs_driver, oidx, NULL);
> > > > +     if (!dev) {
> > > > +             ret = -ENOMEM;
> > > > +             goto fail;
> > > > +     }
> > > > +
> > > > +     vsdev->device = dev;
> > > > +     dev_set_drvdata(dev, vsdev);
> > > > +
> > > > +     /* Create custom sysfs files for this device for events */
> > > > +     ret = sysfs_create_group(&dev->kobj, &vs_info_attr_grp);
> > >
> > > Please no.  You just raced with userspace and lost (i.e. userspace has
> > > no idea these files are present.)
> > >
> > > Please use the correct apis for this, if you _REALLY_ want special sysfs
> > > files for a tty device.
> > Any specific API would you like to suggest. I am unable to progress on
> > how to address this one.
>
> Now that you have moved things to configfs, maybe you do not need the
> sysfs files anymore?
>
> Ah your "control" sysfs files, ok, you need to set the driver's
> dev_groups variable to point to your sysfs attributes, and then the
> driver core will properly set up these files.
>
> hope this helps,
>
> greg k-h