Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 27 Nov 2019 18:36:41 +0100
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To:     Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Cc:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH] avctp: Set more descriptive name for uinput device
Message-ID: <20191127173641.2zsi7ccsnf3seoev@pali>
References: <20191126212753.18455-1-pali.rohar@gmail.com>
 <CABBYNZ+X1xThm9rrcj7ny9f9r0sVs-fTR2JroQdRgZTRWLcN6g@mail.gmail.com>
 <20191127160720.aleatv6l4swopg57@pali>
 <CANFp7mW47eJ6dquzLYX-uKoAO=Qc=ObgmSWQ5U0Xs7ThEV0Urg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="mz3nyjpvo2oldgpb"
Content-Disposition: inline
In-Reply-To: <CANFp7mW47eJ6dquzLYX-uKoAO=Qc=ObgmSWQ5U0Xs7ThEV0Urg@mail.gmail.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk


--mz3nyjpvo2oldgpb
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wednesday 27 November 2019 09:21:24 Abhishek Pandit-Subedi wrote:
> On Wed, Nov 27, 2019 at 8:07 AM Pali Roh=C3=A1r <pali.rohar@gmail.com> wr=
ote:
> >
> > Hi!
> >
> > On Wednesday 27 November 2019 17:50:40 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Tue, Nov 26, 2019 at 11:31 PM Pali Roh=C3=A1r <pali.rohar@gmail.co=
m> wrote:
> > > >
> > > > ---
> > > >  profiles/audio/avctp.c | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
> > > > index 70a3e40b2..ae53587ad 100644
> > > > --- a/profiles/audio/avctp.c
> > > > +++ b/profiles/audio/avctp.c
> > > > @@ -1181,8 +1181,7 @@ static int uinput_create(char *name)
> > > >         }
> > > >
> > > >         memset(&dev, 0, sizeof(dev));
> > > > -       if (name)
> > > > -               strncpy(dev.name, name, UINPUT_MAX_NAME_SIZE - 1);
> > > > +       snprintf(dev.name, sizeof(dev.name), "Bluetooth Audio/Video=
 Remote Control%s%s", name ? " " : "", name ? name : "");
> > > >
> > > >         dev.id.bustype =3D BUS_BLUETOOTH;
> > > >         dev.id.vendor  =3D 0x0000;
> > > > --
> > > > 2.11.0
> > >
> > > It is already setting a bustype to BUS_BLUETOOTH why do you need to
> > > make it more specific,
> >
> > Because more tools shows only device name. Also in kernel dmesg is only
> > device name. And MAC address is really something which does not say
> > anything about device...
> >
> > For this reason also older input devices (implemented in kernel) exports
> > more descriptive names, e.g. "AT Translated Set 2 keyboard" or "ETPS/2
> > Elantech TrackPoint" or "Integrated IR Camera".
> >
> > > btw it needs to be limited to UINPUT_MAX_NAME_SIZE.
> >
> > Is not this implicitly defined by sizeof()?
> >
> > > Id say if we want to make it declare the connection type
> >
> > Yes, connection type would be really useful.
> >
> > Now I'm playing with exporting "button press even" from HSP profile via
> > uinput device too and therefore connection type should be there.
> >
> > > that probably something that needs to be encoded in
> > > the bus itself, like BUS_BLUETOOTH_AVCTP, BUS_BLUETOOTH_HOG, etc.
> >
> > But bus type is kernel API and there is no such AVCTP or HOG. So we need
> > to stick with BUS_BLUETOOH.
> >
> > Other kernel devices also put protocol information into name (e.g.
> > touchpad / trackpoints), so it is ideal place also for bluetooth
> > devices.
> >
> > And probably most userspace devices are mapped to BUS_VIRTUAL. There
> > are no subtypes.
> >
> > --
> > Pali Roh=C3=A1r
> > pali.rohar@gmail.com
>=20
> Wouldn't it be better to use the device name for uinput rather than the a=
ddress?

Hi!

Can we access device name in bluez? If yes that would be improvement too!

But because more profiles can export input device we still should
include profile name (or abbrev) into device name.

> In `init_uinput` (avctp.c), uinput_create is called with
> device_get_address instead of device_get_name. It would probably be
> better to use the device name for the uinput and set the device
> address as the `uniq` attribute (as it is done for /dev/uhid for HID
> devices).

And how to set uniq attribute? I'm looking at uinput API, but do not see
any way how to set such thing.

According to uinput kernel documentation there are two ways how to setup
uinput device. New one via UI_DEV_SETUP ioctl and the old one by writing
structure via write() syscall.

https://www.kernel.org/doc/html/latest/input/uinput.html

New way which has to call UI_DEV_SETUP ioctl has following struct
uinput_setup members:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/inc=
lude/uapi/linux/uinput.h#n67
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/inc=
lude/uapi/linux/input.h#n58

  u16 bustype;
  u16 vendor;
  u16 product;
  u16 version;
  char name[UINPUT_MAX_NAME_SIZE];
  u32 ff_effects_max;

Old way how to setup uinput is to call write() systecall with struct
uinput_user_dev members, which are:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/inc=
lude/uapi/linux/uinput.h#n223

  char name[UINPUT_MAX_NAME_SIZE];
  u16 bustype;
  u16 vendor;
  u16 product;
  u16 version;
  u32 ff_effects_max;
  s32 absmax[ABS_CNT];
  s32 absmin[ABS_CNT];
  s32 absfuzz[ABS_CNT];
  s32 absflat[ABS_CNT];

And I do not see any field which could represents that "uniq" attribute.
Seems that it can be exported only by kernel input drivers, not uinput
userspace drivers...

> So in the following, replace ATTR{name} with something like "Some
> Company Headphones", ATTR{uniq} =3D "33:33:33:ff:ff:ff" and ATTR{phys}
> with the controller's address.
>=20
> $ udevadm info -a -p /sys/devices/virtual/input/input21/
> ...
>   looking at device '/devices/virtual/input/input21':
>     KERNEL=3D=3D"input21"
>     SUBSYSTEM=3D=3D"input"
>     DRIVER=3D=3D""
>     ATTR{inhibited}=3D=3D"0"
>     ATTR{name}=3D=3D"33:33:33:FF:FF:FF"
>     ATTR{phys}=3D=3D""
>     ATTR{properties}=3D=3D"0"
>     ATTR{uniq}=3D=3D""
>=20
> This is what uhid looks like in comparison:
>=20
> $ udevadm info -a -p /sys/class/misc/uhid/0005\:046D\:B33B.0002/input/inp=
ut18/
>=20
>   looking at device
> '/devices/virtual/misc/uhid/0005:046D:B33B.0002/input/input18':
>     KERNEL=3D=3D"input17"
>     SUBSYSTEM=3D=3D"input"
>     DRIVER=3D=3D""
>     ATTR{inhibited}=3D=3D"0"
>     ATTR{name}=3D=3D"Keyboard K780 Keyboard"
>     ATTR{phys}=3D=3D"ab:cd:ef:12:34:56"
>     ATTR{properties}=3D=3D"0"
>     ATTR{uniq}=3D=3D"33:33:33:33:ff:ff"
>=20
>=20
> Abhishek

--=20
Pali Roh=C3=A1r
pali.rohar@gmail.com

--mz3nyjpvo2oldgpb
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQS4VrIQdKium2krgIWL8Mk9A+RDUgUCXd60JwAKCRCL8Mk9A+RD
Ui4NAJ9pc/Y0V92BC9RZFroimf+fNU9ULQCcCEM1NumNEn7ObhejOSAPlnPpfBY=
=v7B8
-----END PGP SIGNATURE-----

--mz3nyjpvo2oldgpb--