On Thu, Oct 01, 2020 at 03:50:09PM +0200, Lars Poeschel wrote: > On Thu, Oct 01, 2020 at 01:24:49PM +0200, Greg Kroah-Hartman wrote: > > On Thu, Oct 01, 2020 at 11:05:31AM +0200, Lars Poeschel wrote: > > > On Wed, Sep 30, 2020 at 11:41:46AM +0200, Uwe Kleine-König wrote: > > > > Hello, > > > > > > > > I added Greg Kroah-Hartman who I discussed this with via irc a bit to > > > > Cc:. > > > > > > > > On Wed, Sep 30, 2020 at 11:20:56AM +0200, Lars Poeschel wrote: > > > > > thank you for your review! > > > > > > > > > > On Wed, Sep 30, 2020 at 08:57:26AM +0200, Uwe Kleine-König wrote: > > > > > > On Tue, Sep 29, 2020 at 02:19:53PM +0200, poesc...@lemonage.de > > > > > > wrote: > > > > > > > From: Lars Poeschel <poesc...@lemonage.de> > > > > > > > > > > > > > > This adds a class to exported pwm devices. > > > > > > > Exporting a pwm through sysfs did not yield udev events. The > > > > > > > > > > > > I wonder what is your use-case here. This for sure also has a place > > > > > > to > > > > > > be mentioned in the commit log. I suspect there is a better way to > > > > > > accomplish you way. > > > > > > > > > > Use-case is to be able to use a pwm from a non-root userspace process. > > > > > I use udev rules to adjust permissions. > > > > > > > > Hmm, how do you trigger the export? Without being aware of all the > > > > details in the sysfs code I would expect that the exported stuff is > > > > available instantly once the write used to export the PWM is completed. > > > > So changing the permissions can be done directly after triggering the > > > > export in the same process. > > > > > > The export is triggered through the userspace process itself. Why can it > > > do this ? Because there is another udev rule, that changes permissions > > > when a pwmchip appears. > > > Then I'd like to have the second udev rule, that changes permissions on > > > the freshly exported pwm. The userspace process can't do this. > > > You are right I could propably do everything from within udev: If a > > > pwmchip appears, export certain pwms and right away change their > > > permissions. It does not also not feel right. It'd require knowledge > > > from the userspace application to be mapped to udev. > > > > The way the kernel code is now, yes, you will not have any way to > > trigger it by userspace as the kernel is creating a "raw" struct device > > that isn't assigned to anything. That is what needs to be fixed here. > > I did a first try with our approach. > I set the class of the child to its parent class. This does work and > create the directories right under /sys/pwm but because the child now > also inherits the dev_groups from the parent its directory also contain > export, unexport and npwm files, that don't apply for pwm's as soon a I > register the device to driver core. > > Did we miss something or is there a way to avoid that ? I had a look at > device_add and saw that as soon as a class it set it's dev_groups get > exported through device_add_attrs.
Ah, you need to tweak that group to only show up for a specific "type" of device. There is a is_visable callback for a group that should be used for this, and you can check the type of device being affected here, try messing with that. And make sure you set a new type for the new devices you are creating. I know that's vague, if you need more help I can work on it next week. > > > > Out of interest: What do you use the pwm for? Isn't there a suitable > > > > kernel driver that can do the required stuff? Compared to the kernel-API > > > > the sysfs interface isn't atomic. Is this an annoyance? > > > > > > Use-case is generating a voltage from the pwm. This voltage is used to > > > signal different states and does not change very often. This is > > > absolutely not annoying that this is not atomic. We just change the duty > > > cycle on the fly. Everything else is configured one time at startup. > > > I'd call what I need pwm-dac. I could not find a ready to use driver. > > > Maybe I could misuse some kernel driver for this. Maybe I could use > > > pwm-led or pwm-brightness or pwm-fan. Propably pwm-regulator could work, > > > there is even a userspace facing part for this, but this is not > > > devicetree ready. > > > ...and the worst, please don't blame me: The application is java, so > > > ioctl is a problem. > > > > I thought java could do ioctls, otherwise how would it ever be able to > > talk to serial ports? > > It is not impossible but it is horrible. java itself can access the > ports through normal file I/O, but it can not set control lines or > baudrate or anything. You need some C-code for this, that is not part of > the java vm itself and has to be called through something called JNI - > java native interface. Ah, ok, yeah, that's not ok, sorry to hear you are stuck with Java :( greg k-h