Page MenuHomeFreeBSD

Delete gpio flags because of not support device. This is same as gpioiic. Add spi clock low delay. Reformat message. ar5315_spi.c spibus minor number fix.
ClosedPublic

Authored by yamori813_yahoo.co.jp on Dec 11 2016, 8:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 6:17 AM
Unknown Object (File)
Feb 11 2024, 10:51 AM
Unknown Object (File)
Jan 9 2024, 5:49 AM
Unknown Object (File)
Jan 9 2024, 5:49 AM
Unknown Object (File)
Jan 9 2024, 5:49 AM
Unknown Object (File)
Jan 9 2024, 5:49 AM
Unknown Object (File)
Jan 9 2024, 4:23 AM
Unknown Object (File)
Dec 22 2023, 11:22 PM
Subscribers

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6207
Build 6455: arc lint + arc unit

Event Timeline

yamori813_yahoo.co.jp retitled this revision from to Delete gpio flags because of not support device. This is same as gpioiic. Add spi clock low delay. Reformat message. ar5315_spi.c spibus minor number fix..
yamori813_yahoo.co.jp updated this object.
yamori813_yahoo.co.jp edited the test plan for this revision. (Show Details)
sys/dev/gpio/gpiospi.c
159

If driver do not support PullUp/PullDown, then why it makes problem?

gpiobus_pin_setflags() in gpibus.c have check gpio_check_flags(). This code check pin capabirity. If low driver not support GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN return EINVAL.

gpiobus_pin_setflags() in gpibus.c have check gpio_check_flags(). This code check pin capabirity. If low driver not support GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN return EINVAL.

Then better to set it as supported by driver, and ignore them in driver. Otherwise other controllers who support pullup/pulldown will have unpredictable results on that lines, because nobody said to pull it.

I seem GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN not set in any sys/mips/ gpio code. And gpioiic.c not use GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN that is same bitbang code . I think better not use in gpiospi.c rather than low driver ignore.

If ignore GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN in low driver then GPIO_PIN_PULLUP/GPIO_PIN_PULLDOWN flag have no meaning in a system.

mizhka edited edge metadata.

Thanks for your contribution!

Generally speaking, it looks fine.
To satisfy both cases (with and wo pulling) it may be worth to make pulling configurable (and make default is off).

This revision is now accepted and ready to land.Dec 12 2016, 6:48 AM

@yamori813_yahoo.co.jp with i2c the bus hardware must have pullups (required by standard). This is why I haven't added it to gpioiic (but maybe I should...).

It is safe to ignore pullup/pulldown in gpio driver, they will have no meaning for that specific gpio controller, if you have a different controller they can still be used.

The check in gpio_check_flags() is wrong, I going to fix it. Thanks!

loos requested changes to this revision.Dec 12 2016, 6:44 PM
loos added a reviewer: loos.
loos added inline comments.
sys/dev/gpio/gpiospi.c
316

The delays still seem wrong to me, see: https://en.wikipedia.org/wiki/Serial_Peripheral_Interface_Bus

We have to take the phase into account.

sys/dev/spibus/spibus.c
120–122

Correct.

sys/mips/atheros/ar531x/ar5315_spi.c
125

Correct, please commit.

This revision now requires changes to proceed.Dec 12 2016, 6:44 PM

I check this code on my develop SPI module code that is ksz8995ma ether switch chip and FON2100. This case delay is fixed problem and work. I check signal enclose picture. If no delay clock is not to zero.

nodelay.jpg (460×660 px, 58 KB)

delay.jpg (460×660 px, 70 KB)

This is my setting hints.

hint.gpiospi.0.at="gpiobus0"
hint.gpiospi.0.pins=0x9a
hint.gpiospi.0.cs0=0
hint.gpiospi.0.sclk=3
hint.gpiospi.0.mosi=1
hint.gpiospi.0.miso=2
hint.gpiospi.0.freq=100000000

yeah, this delay() fixes the clock output, but what I am saying is that data and clock should come out with a 90o phase difference. your patch doesn't not fix this but it is surely an improvement (you can commit this change too).

yamori813_yahoo.co.jp edited edge metadata.
  • back to original code at GPIOBUS_PIN_SETFLAG. Because of gpiobus.c fixed
ray edited edge metadata.
This revision was automatically updated to reflect the committed changes.

Why delete my added gpio_delay() in gpiospi.c ? My KSZ8995MA code don't work no gpio_delay() on my target. I think current code clock is wrong.

Why delete my added gpio_delay() in gpiospi.c ? My KSZ8995MA code don't work no gpio_delay() on my target. I think current code clock is wrong.

Hi Hiroki-san,

gpio_delay has been committed by next commit in sequence: https://github.com/freebsd/freebsd/commit/1ea50c0b5014d04d2b8586d6efd258abe411d87f

I suppose Phabricator doesn't track commits for already closed reviews (smells like possible improvement ;).