Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpiConnectionSettings should have a property for chip select active state #201

Closed
shaggygi opened this issue Feb 2, 2019 · 8 comments
Closed
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins

Comments

@shaggygi
Copy link
Contributor

shaggygi commented Feb 2, 2019

This proposal is to add a chip select active state property to the SpiConnectionSettings class.

Rationale and Usage

While most SPI devices are selected with an active-low chip select pin, there are devices that are active-high. To overcome this today (to my current knowledge) is adding additional hardware logic to inverse the select line between master/slave devices.

Proposed Changes

The proposed change is to add a new property for a chip select active state.

  • This could be named ChipSelectActiveHigh or similar. It could have a default value of false that represents active-high since this is the minority use.
  • It also doesn't have to be included in the SpiConnectionSettings constructor as is similar to other properties like ClockFrequency, Mode, etc..
var spiConnectionSettings = new SpiConnectionSettings(0, 0)
{
    ChipSelectActiveHigh = true,
    ClockFrequency = 1000000,
    Mode = SpiMode.Mode0
};

References

Open Questions

  1. Windows 10 IoT appears to only allow for active-low state. Is there a way to change this in a system setting or within Windows.Devices.Spi API?
  2. It appears you can change this in Linux, but I have not tested. See https://01.org/linuxgraphics/gfx-docs/drm/driver-api/spi.html. Need to review if this can be performed programmatically or must be hardcoded in SPI settings.
@krwq krwq added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 3, 2019
@krwq krwq changed the title [Proposal] SpiConnectionSettings should have a property for chip select active state SpiConnectionSettings should have a property for chip select active state Feb 3, 2019
@joshfree joshfree added the area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins label Feb 23, 2019
@joshfree joshfree added this to the 3.0 milestone Feb 23, 2019
@joshfree
Copy link
Member

@krwq could you please investigate for 3.0?

@krwq
Copy link
Member

krwq commented Feb 23, 2019

@shaggygi got any specific devices in mind? (just curious)

@shaggygi
Copy link
Contributor Author

@krwq I came across this when trying to use with bit-banging GPIO pins. You can use SPI with a 74595 logic chip. While I have a partial binding started for this chip, it could be used with something like I2C/SPI Character LCD Backpack using SPI (with active-high CS).

While reviewing this topic, I found that many processors allow CS enable to be configured to Low or High. I also came across various SPI tools where they can be configured and blogs/forums where users state they have a device that needs to be active-high enabled. However, there was never a specific device name mentioned where I could verify the CS interface.

I agree most cases will be active-low CS as that appears to be the preferred method. I'm just thinking this option might need to be available sooner than later. Hope this helps.

@joperezr
Copy link
Member

joperezr commented Mar 6, 2019

Given that this doesn't really apply to every device, I think that I would prefer if the active state would be passed in (with a paramter on the constructor) on the binding itself. Most of the devices I've worked with only allow having one state for the CS, so I don't think this should go on the SpiConnection class IMHO.

@krwq
Copy link
Member

krwq commented Mar 6, 2019

@joperezr I think quite opposite - I'd leave the constructors to the absolute requirement you have to set and move everything unusual to settings which in this case would be something really similar to what @shaggygi has proposed (I'd perhaps change bool to the enum but the same idea).

@shaggygi
Copy link
Contributor Author

shaggygi commented Mar 6, 2019

@krwq I like your idea better with an enum. Not sure if you were thinking of a new enum or possibly use PinValue where it would accept a few ways. If so, we might want to use ChipSelectActiveState as the property name and still use Low/false/0 the default.

// By PinValue.Low or PinValue.High
var spiConnectionSettings = new SpiConnectionSettings(0, 0)
{
    ChipSelectActiveState = PinValue.High,
    ClockFrequency = 1000000,
    Mode = SpiMode.Mode0
};

// By false or true
var spiConnectionSettings = new SpiConnectionSettings(0, 0)
{
    ChipSelectActiveState = true,
    ClockFrequency = 1000000,
    Mode = SpiMode.Mode0
};

// By 0 or 1
var spiConnectionSettings = new SpiConnectionSettings(0, 0)
{
    ChipSelectActiveState = 1,
    ClockFrequency = 1000000,
    Mode = SpiMode.Mode0
};

@shaggygi
Copy link
Contributor Author

shaggygi commented Mar 6, 2019

But then again, if we added an enum for #202, we might want to group related enums under the SPI namespace like ChipSelectState and DataFlow or similar?

var spiConnectionSettings = new SpiConnectionSettings(0, 0)
{
    ChipSelectState = ChipSelectState.ActiveHigh,
    DataFlow = DataFlow.MsbFirst,
    ClockFrequency = 1000000,
    Mode = SpiMode.Mode0
};

@krwq
Copy link
Member

krwq commented Mar 6, 2019

Yes this looks more or less correct - once I (or someone else) start working on this we can take a closer look at the exact shape and what makes most sense (probably makes sense to compare to other implementations like python/C and see what they call these so that we call them similarly).

@krwq krwq closed this as completed in #331 Mar 14, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Oct 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Device.Gpio Contains types for using general-purpose I/O (GPIO) pins
Projects
None yet
Development

No branches or pull requests

4 participants